From 2a17081e7f6d49d134ffa104595c6743fcbf9bb4 Mon Sep 17 00:00:00 2001 From: japabu Date: Fri, 9 Oct 2026 12:32:57 +0200 Subject: [PATCH 1/6] A machine's image goes on its one NVMe disk beside its DATA: the Headless guests and `cargo run` boot off it with no stick, and `/log` and `/home` are read back across a reboot `image::install` lays DATA first and every partition of a boot image after it on one disk file, keeping a DATA partition the file already holds, and writes only the image's non-zero blocks. The table writer `create_gpt_disk` and `designate_data_disk` each carried is one function, `lay`. The harness's `Shape` states where a machine's image and DATA are (`Storage`): `Headless` and `HeadlessNoIommu` boot off the disk, a new `HeadlessNoUsb` is the same machine with no USB controller, and `Metal` and the `virt` profiles keep their stick. `cargo run` installs the built image on `target/disk.img` for the x86-64 Virtio and Gop profiles. `nvme_disk_keeps_log_and_home` boots `HeadlessNoUsb`, writes under `/home`, says a line, reboots inside one QEMU and reads both back. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A --- ...-nvme-looks-1500-ms-for-a-usb-boot-disk.md | 37 ++ src/image.rs | 323 ++++++++++++++---- src/qemu.rs | 65 +++- tests/common/qemu.rs | 130 +++++-- tests/toyos.rs | 117 +++++++ 5 files changed, 554 insertions(+), 118 deletions(-) create mode 100644 issues/a-machine-that-boots-off-nvme-looks-1500-ms-for-a-usb-boot-disk.md diff --git a/issues/a-machine-that-boots-off-nvme-looks-1500-ms-for-a-usb-boot-disk.md b/issues/a-machine-that-boots-off-nvme-looks-1500-ms-for-a-usb-boot-disk.md new file mode 100644 index 00000000000..b53c4eac644 --- /dev/null +++ b/issues/a-machine-that-boots-off-nvme-looks-1500-ms-for-a-usb-boot-disk.md @@ -0,0 +1,37 @@ +--- +status: open +kind: defect +opened: 2026-10-09 +--- + +# A machine that boots off NVMe looks 1500 ms for a USB boot disk + +`gpt::probe_usb_disks` (`kernel/src/gpt.rs`) keeps asking the USB disks for +the partition firmware booted from until one carries it or +`xhci::PORT_SETTLE_CEILING`, 1500 ms, has passed. On a machine whose image is +on its NVMe disk no USB disk ever carries it, so every boot spends the whole +ceiling, on the boot CPU, before a task runs: paced by `PORT_POLL` in a +`spin_loop`, with nothing else scheduled. + +Measured on `nvme_disk_keeps_log_and_home`'s machine, which has no USB +controller at all, on both of its boots: the kernel says `xHCI: no controller +on this machine, USB input unavailable` and, stamped 1501 ms later (`2.917` +and `4.418` on the test's first run), `usb-storage: 0 disk(s) on this machine +and none carries the boot partition after 1500 ms of looking`. Those boots +report `kernel to Boot: complete 1741 ms`. Every machine that boots off its +NVMe disk pays it, `cargo run`'s among them. + +The kernel already knows the outcome it waits for is impossible on a machine +with no xHCI controller, and it is told nothing about which bus the boot +partition's disk is on: the loader has the device path and hands over the +partition's GUID and extent alone. + +**Exit condition.** A machine that boots off a disk this kernel does not +drive spends none of that ceiling: the line above is gone from +`nvme_disk_keeps_log_and_home`'s boots, or the wait is deleted with the +kernel's xHCI at step 10 of +`issues/the-kernel-is-small-interrupts-post-and-threads-wait.md`. + +## Owner + +`kernel/src/gpt.rs`; unheld. diff --git a/src/image.rs b/src/image.rs index 5ef1abdbfaa..4a5146b0f4c 100644 --- a/src/image.rs +++ b/src/image.rs @@ -1,5 +1,5 @@ use std::collections::BTreeMap; -use std::io::{Cursor, Read, Seek, SeekFrom}; +use std::io::{Cursor, Read, Seek, SeekFrom, Write}; use std::num::NonZeroU64; use std::path::Path; @@ -691,8 +691,6 @@ const TOYOS_BOOT: gpt::partition_types::Type = gpt::partition_types::Type { /// file stays sparse. The stamp names that partition's own block count, so a /// copy of this image designates no partition of another size. pub fn designate_data_disk(path: &Path, len: u64) -> (u64, u64) { - use std::io::Write; - let Some(data_bytes) = len .checked_sub(2 * PARTITION_ALIGN as u64) .map(|b| b / SECTOR as u64 * SECTOR as u64) @@ -700,43 +698,111 @@ pub fn designate_data_disk(path: &Path, len: u64) -> (u64, u64) { else { panic!("a {len}-byte disk has no room for a DATA partition"); }; - - let mut file = std::fs::OpenOptions::new() + let file = std::fs::OpenOptions::new() .read(true) .write(true) .open(path) .unwrap_or_else(|e| panic!("open {} to partition it: {e}", path.display())); - let mbr = - gpt::mbr::ProtectiveMBR::with_lb_size(u32::try_from(len / 512 - 1).unwrap_or(0xFF_FF_FF_FF)); - mbr.overwrite_lba0(&mut file).expect("write the protective MBR"); + let (mut device, starts) = lay(Box::new(file), len, &[data_row(uuid::Uuid::new_v4(), data_bytes)]); + designate(&mut device, starts[0], data_bytes); + device.flush().expect("flush the data disk"); + (starts[0], data_bytes) +} - let mut gdisk = gpt::GptConfig::default() - .initialized(false) - .writable(true) - .logical_block_size(gpt::disk::LogicalBlockSize::Lb512) - .create_from_device(Box::new(file), None) - .expect("create a GPT on the data disk"); - gdisk - .update_partitions(BTreeMap::::new()) - .expect("initialize the data disk's partition table"); - let id = gdisk - .add_partition("ToyOS data", data_bytes, TOYOS_DATA, 0, Some((PARTITION_ALIGN / 512) as u64)) - .expect("add the data partition"); - let placed = gdisk.partitions().get(&id).expect("the partition was just added"); - let start = placed - .bytes_start(gpt::disk::LogicalBlockSize::Lb512) - .expect("the data partition's start"); - let bytes = placed - .bytes_len(gpt::disk::LogicalBlockSize::Lb512) - .expect("the data partition's length"); - assert_eq!(start % SECTOR as u64, 0, "the data partition starts at byte {start}"); - assert_eq!(bytes % SECTOR as u64, 0, "the data partition is {bytes} bytes"); - - let mut device = gdisk.write().expect("write the data disk's GPT"); - device.seek(SeekFrom::Start(start)).expect("seek to the data partition"); +/// A TOYOS-DATA partition's entry. +fn data_row(guid: uuid::Uuid, len: u64) -> Row { + Row { what: "DATA partition".into(), name: "ToyOS data".into(), kind: TOYOS_DATA, guid, len } +} + +/// Stamp the designation on the `bytes`-long partition at byte `at` of `device`. +fn designate(device: &mut gpt::DiskDeviceObject<'_>, at: u64, bytes: u64) { + device.seek(SeekFrom::Start(at)).expect("seek to the data partition"); device.write_all(&designation(bytes / SECTOR as u64)).expect("stamp the data partition"); - device.flush().expect("flush the data disk"); - (start, bytes) +} + +/// What a machine's own disk holds once the boot image `image` is installed +/// on the file at `disk`: DATA, `data_bytes` of it, and after it every +/// partition of the image under the name, type and GUID the image gave it. +/// +/// **A disk that already holds that DATA partition keeps it**, its bytes and +/// its GUID, and the image is replaced around it; any other file is made anew +/// and its DATA designated. DATA comes first so that no image's size moves it. +/// Only the tables, the stamp and the image's blocks that are not zero are +/// written, so the file costs the host what the image does. +pub fn install(image: &[u8], disk: &Path, data_bytes: u64) { + use gpt::disk::LogicalBlockSize::Lb512; + let mut listed = [None; 16]; + let scan = toyos_gpt::list(&mut ImageSectors(image), &mut listed) + .unwrap_or_else(|e| panic!("the image to install carries no partition table: {e:?}")); + assert!(scan.matched as usize <= listed.len(), "the image to install carries {} partitions", scan.matched); + // The names, which the reader above does not read; the types, which this one does not keep. + let mut names: BTreeMap = gpt::header::read_header_from_arbitrary_device(&mut Cursor::new(image), Lb512) + .and_then(|header| gpt::partition::file_read_partitions(&mut Cursor::new(image), &header, Lb512)) + .unwrap_or_else(|e| panic!("the image to install carries no readable partition entries: {e}")) + .into_values() + .map(|part| (part.part_guid, part.name)) + .collect(); + let carried: Vec<(Row, &[u8])> = listed + .iter() + .flatten() + .map(|entry| { + let part = entry.unwrap_or_else(|unplaced| panic!("the image to install states no partition: {unplaced:?}")); + let guid = uuid::Uuid::from_bytes_le(part.unique_guid().0); + let kind = [gpt::partition_types::EFI, gpt::partition_types::BASIC, TOYOS_SLOTS, TOYOS_BOOT, TOYOS_ROOT] + .into_iter() + .find(|kind| uuid::Uuid::parse_str(kind.guid).is_ok_and(|text| text.to_bytes_le() == part.type_guid().0)) + .unwrap_or_else(|| panic!("partition {guid} is of type {}, which no boot image carries", part.type_guid())); + let name = names.remove(&guid).unwrap_or_else(|| panic!("partition {guid} has no name")); + let at = part.first_lba() as usize * LBA as usize; + let bytes = &image[at..at + part.lba_count().get() as usize * LBA as usize]; + (Row { what: format!("installed {name}"), name, kind, guid, len: bytes.len() as u64 }, bytes) + }) + .collect(); + + let kept = data_on(disk, data_bytes); + let (mut rows, volumes): (Vec, Vec<&[u8]>) = carried.into_iter().unzip(); + rows.insert(0, data_row(kept.unwrap_or_else(uuid::Uuid::new_v4), data_bytes)); + let total = disk_bytes(rows.iter().map(|row| row.len)); + + let mut file = std::fs::OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(kept.is_none()) + .open(disk) + .unwrap_or_else(|e| panic!("open {} to install on it: {e}", disk.display())); + // Cut to DATA's end and grown again, with the old table's megabyte + // cleared: everything but DATA reads zero from here. + let data_end = PARTITION_ALIGN as u64 + data_bytes; + file.set_len(data_end) + .and_then(|()| file.set_len(total)) + .and_then(|()| file.write_all(&vec![0u8; PARTITION_ALIGN])) + .unwrap_or_else(|e| panic!("clear {} around its DATA: {e}", disk.display())); + + let (mut device, starts) = lay(Box::new(file), total, &rows); + assert_eq!(starts[0], PARTITION_ALIGN as u64, "DATA is not the disk's first partition"); + if kept.is_none() { + designate(&mut device, starts[0], data_bytes); + } + for ((row, volume), start) in rows[1..].iter().zip(volumes).zip(&starts[1..]) { + for (i, block) in volume.chunks(PARTITION_ALIGN).enumerate().filter(|(_, block)| block.iter().any(|byte| *byte != 0)) { + let at = start + (i * PARTITION_ALIGN) as u64; + device + .seek(SeekFrom::Start(at)) + .and_then(|_| device.write_all(block)) + .unwrap_or_else(|e| panic!("write the {} at byte {at}: {e}", row.what)); + } + } + device.flush().unwrap_or_else(|e| panic!("flush {}: {e}", disk.display())); +} + +/// The unique GUID of the DATA partition [`install`] put on the file at +/// `disk`, where it holds one of `data_bytes`: the one an install keeps. +fn data_on(disk: &Path, data_bytes: u64) -> Option { + let mut file = std::fs::File::open(disk).ok()?; + let part = only_partition(&mut FileSectors(&mut file), toyos_gpt::Guid::TOYOS_DATA).ok()?; + let (start, len) = (part.first_lba() * u64::from(LBA), part.lba_count().get() * u64::from(LBA)); + (start == PARTITION_ALIGN as u64 && len == data_bytes).then(|| uuid::Uuid::from_bytes_le(part.unique_guid().0)) } /// Block 0 of a volume: the magic and its block count. @@ -797,32 +863,41 @@ impl Part { } } -fn create_gpt_disk(parts: &[Part]) -> Vec { - // `add_partition` places each partition itself; this is the size the disk - // has to be for it to have somewhere to put them — an aligned gap before - // the first, each one aligned after the last, and a gap after the last for - // the backup table. - let mut end = PARTITION_ALIGN; - for part in parts { - assert!(part.bytes.len() as u64 <= part.len, "the {} is {} bytes in a {}-byte partition", part.what, part.bytes.len(), part.len); - end = align_up(end, PARTITION_ALIGN) + part.len as usize; - } - let total_size = round_up_sectors(align_up(end, PARTITION_ALIGN) + PARTITION_ALIGN); - assert_eq!(total_size % 512, 0, "image must be a whole number of 512-byte sectors to be flashable"); - let mut disk = vec![0u8; total_size]; +/// A partition's entry in a table: what it is called in a refusal and in the +/// table, its type and unique GUID, and its length. +struct Row { + what: String, + name: String, + kind: gpt::partition_types::Type, + guid: uuid::Uuid, + len: u64, +} - let mut cursor = Cursor::new(&mut disk); +/// The size a disk has to be for [`lay`] to place partitions of these lengths +/// on it: an aligned gap before the first, each one aligned after the last, +/// and a gap after the last for the backup table. +fn disk_bytes(lens: impl Iterator) -> u64 { + let end = lens.fold(PARTITION_ALIGN, |end, len| align_up(end, PARTITION_ALIGN) + len as usize); + round_up_sectors(align_up(end, PARTITION_ALIGN) + PARTITION_ALIGN) as u64 +} - let mbr = gpt::mbr::ProtectiveMBR::with_lb_size( - u32::try_from((total_size / 512) - 1).unwrap_or(0xFF_FF_FF_FF), - ); - mbr.overwrite_lba0(&mut cursor).expect("failed to write MBR"); +/// Write a protective MBR and a GPT on `device`, `total` bytes long, placing +/// `rows` in their order, each on a [`PARTITION_ALIGN`] boundary and under +/// its own GUID: the device, and the byte each row starts at. +fn lay<'a>( + mut device: gpt::DiskDeviceObject<'a>, + total: u64, + rows: &[Row], +) -> (gpt::DiskDeviceObject<'a>, Vec) { + assert_eq!(total % 512, 0, "image must be a whole number of 512-byte sectors to be flashable"); + let mbr = gpt::mbr::ProtectiveMBR::with_lb_size(u32::try_from(total / 512 - 1).unwrap_or(0xFF_FF_FF_FF)); + mbr.overwrite_lba0(&mut device).expect("failed to write MBR"); let mut gdisk = gpt::GptConfig::default() .initialized(false) .writable(true) .logical_block_size(gpt::disk::LogicalBlockSize::Lb512) - .create_from_device(Box::new(cursor), None) + .create_from_device(device, None) .expect("failed to create GPT disk"); gdisk @@ -830,46 +905,47 @@ fn create_gpt_disk(parts: &[Part]) -> Vec { .expect("failed to initialize partition table"); let align = Some((PARTITION_ALIGN / 512) as u64); - let ids: Vec = parts + let ids: Vec = rows .iter() - .map(|part| { + .map(|row| { gdisk - .add_partition(part.name, part.len, part.kind.clone(), 0, align) - .unwrap_or_else(|e| panic!("failed to add the {}: {e}", part.what)) + .add_partition(&row.name, row.len, row.kind.clone(), 0, align) + .unwrap_or_else(|e| panic!("failed to add the {}: {e}", row.what)) }) .collect(); - // The GUID `add_partition` drew for each is discarded for the one this - // image already wrote down: the log's on the ESP, every slot partition's in - // the slot table, and the ESP's own because a firmware boot entry names a + // The GUID `add_partition` drew for each is discarded for the one its row + // names: the log's is written on the ESP, every slot partition's in the + // slot table, and the ESP's own because a firmware boot entry names a // partition by GUID and every name is drawn once. let mut table = gdisk.partitions().clone(); - for (part, id) in parts.iter().zip(&ids) { - table.get_mut(id).expect("a partition that was just added").part_guid = part.guid; + for (row, id) in rows.iter().zip(&ids) { + table.get_mut(id).expect("a partition that was just added").part_guid = row.guid; } gdisk.update_partitions(table).expect("failed to stamp the partitions' unique GUIDs"); - let placed: Vec<(&str, usize, usize)> = parts + let placed: Vec<(&str, u64, u64)> = rows .iter() .zip(&ids) - .map(|(part, id)| { + .map(|(row, id)| { let entry = gdisk.partitions().get(id).expect("a partition that was just added"); - let start = entry.bytes_start(gpt::disk::LogicalBlockSize::Lb512).expect("a partition's start") as usize; + let start = entry.bytes_start(gpt::disk::LogicalBlockSize::Lb512).expect("a partition's start"); let len = entry.bytes_len(gpt::disk::LogicalBlockSize::Lb512).expect("a partition's length"); // A slot's ROOT is read for exactly its signed length, and a // partition the table made any other length than asked is one // whose contents nothing here placed. - assert_eq!(len, part.len, "the table gives the {} {len} bytes for a {}-byte partition", part.what, part.len); - (part.what, start, part.len as usize) + assert_eq!(len, row.len, "the table gives the {} {len} bytes for a {}-byte partition", row.what, row.len); + (row.what.as_str(), start, row.len) }) .collect(); // The invariant [`PARTITION_ALIGN`] exists for, checked rather than // assumed: the kernel mounts several at once over one 4 KiB block device, // and a device block belonging to two volumes would be cached twice. + let sector = SECTOR as u64; for (what, start, len) in &placed { - assert_eq!(start % SECTOR, 0, "the {what} starts at byte {start}, off a {SECTOR}-byte block"); - assert_eq!(len % SECTOR, 0, "the {what} is {len} bytes, not whole {SECTOR}-byte blocks"); + assert_eq!(start % sector, 0, "the {what} starts at byte {start}, off a {SECTOR}-byte block"); + assert_eq!(len % sector, 0, "the {what} is {len} bytes, not whole {SECTOR}-byte blocks"); } let mut by_start = placed.clone(); by_start.sort_by_key(|(_, start, _)| *start); @@ -884,14 +960,29 @@ fn create_gpt_disk(parts: &[Part]) -> Vec { ); } - let mut disk_device = gdisk.write().expect("failed to write GPT"); + let device = gdisk.write().expect("failed to write GPT"); + (device, placed.iter().map(|(_, start, _)| *start).collect()) +} + +fn create_gpt_disk(parts: &[Part]) -> Vec { + for part in parts { + assert!(part.bytes.len() as u64 <= part.len, "the {} is {} bytes in a {}-byte partition", part.what, part.bytes.len(), part.len); + } + let rows: Vec = parts + .iter() + .map(|part| Row { what: part.what.into(), name: part.name.into(), kind: part.kind.clone(), guid: part.guid, len: part.len }) + .collect(); + let total_size = disk_bytes(rows.iter().map(|row| row.len)); + let mut disk = vec![0u8; total_size as usize]; + let (mut disk_device, starts) = lay(Box::new(Cursor::new(&mut disk)), total_size, &rows); disk_device.seek(std::io::SeekFrom::Start(0)).expect("failed to seek"); - let mut final_bytes = vec![0u8; total_size]; + let mut final_bytes = vec![0u8; total_size as usize]; disk_device.read_exact(&mut final_bytes).expect("failed to read disk"); - for (part, (_, start, _)) in parts.iter().zip(&placed) { - final_bytes[*start..*start + part.bytes.len()].copy_from_slice(&part.bytes); + for (part, start) in parts.iter().zip(&starts) { + let start = *start as usize; + final_bytes[start..start + part.bytes.len()].copy_from_slice(&part.bytes); } let judged: Vec<(&str, toyos_gpt::Guid, Volume)> = parts @@ -1186,6 +1277,92 @@ mod tests { assert_eq!(parts.signed, &signed); } + /// Every partition of `image` is on `disk` under its GUID with its bytes, + /// found by the reader the kernel and `diskserver` use. + fn carries_every_partition_of(disk: &mut std::fs::File, image: &[u8]) { + let mut listed = [None; 16]; + toyos_gpt::list(&mut ImageSectors(image), &mut listed).expect("the image's table"); + let parts: Vec = listed.iter().flatten().map(|entry| entry.expect("a placed entry")).collect(); + assert!(parts.len() >= 5, "an image of {} partitions", parts.len()); + for part in parts { + let at = part.first_lba() as usize * LBA as usize; + let len = part.lba_count().get() as usize * LBA as usize; + let on_disk = toyos_gpt::locate(&mut FileSectors(disk), part.unique_guid()).expect("the partition on the disk").partition(); + assert_eq!(on_disk.type_guid(), part.type_guid(), "{}", part.unique_guid()); + assert_eq!(on_disk.lba_count(), part.lba_count(), "{}", part.unique_guid()); + let mut installed = vec![0u8; len]; + disk.seek(SeekFrom::Start(on_disk.first_lba() * u64::from(LBA))) + .and_then(|_| disk.read_exact(&mut installed)) + .expect("read it back"); + assert!(installed == image[at..at + len], "{} was installed as other bytes", part.unique_guid()); + } + } + + /// The one DATA partition on `disk`: its GUID, its extent and its first block. + fn data_of(disk: &mut std::fs::File) -> (toyos_gpt::Guid, u64, u64, Vec) { + let part = only_partition(&mut FileSectors(disk), toyos_gpt::Guid::TOYOS_DATA).expect("one DATA partition"); + let (start, bytes) = partition_extent(disk, part.unique_guid().0).expect("DATA's extent"); + let mut first = vec![0u8; SECTOR]; + disk.seek(SeekFrom::Start(start)).and_then(|_| disk.read_exact(&mut first)).expect("read DATA"); + (part.unique_guid(), start, bytes, first) + } + + /// An install puts a designated DATA first and the image after it; a + /// second install, of an image of another size, replaces the image and + /// leaves DATA where it was, under its GUID, holding what was written to + /// it; and a DATA of another size is a disk made anew. + #[test] + fn an_install_replaces_the_image_around_the_data_it_keeps() { + const DATA: u64 = 4 << 20; + let key = key(); + let root = tiny_root(); + let dir = toyos_tmpdir::TempDir::new("image-install"); + let path = dir.join("disk.img"); + let open = || std::fs::OpenOptions::new().read(true).write(true).open(&path).expect("open the disk"); + + let first = create_boot_image(Arch::X86_64, b"kernel", b"bootloader", &root, "", signing(&key), None); + install(&first, &path, DATA); + let mut disk = open(); + carries_every_partition_of(&mut disk, &first); + let (guid, start, bytes, stamp) = data_of(&mut disk); + assert_eq!((start, bytes), (PARTITION_ALIGN as u64, DATA)); + assert_eq!(stamp, designation(DATA / SECTOR as u64)); + + // What a volume's format does to the stamp, and a block at DATA's end. + let written = vec![0xA5u8; SECTOR]; + for at in [start, start + bytes - SECTOR as u64] { + disk.seek(SeekFrom::Start(at)).and_then(|_| disk.write_all(&written)).expect("write DATA"); + } + drop(disk); + + let second = create_boot_image( + Arch::X86_64, + b"a longer kernel", + b"bootloader", + &root, + "", + signing(&key), + Some(SecondSlot { root_bytes: 8 << 20 }), + ); + assert_ne!(first.len(), second.len()); + install(&second, &path, DATA); + let mut disk = open(); + carries_every_partition_of(&mut disk, &second); + assert_eq!(data_of(&mut disk), (guid, start, bytes, written.clone())); + let mut last = vec![0u8; SECTOR]; + disk.seek(SeekFrom::Start(start + bytes - SECTOR as u64)).and_then(|_| disk.read_exact(&mut last)).expect("read DATA"); + assert_eq!(last, written); + let gone = only(&first, toyos_gpt::Guid::EFI_SYSTEM); + assert!(partition_extent(&mut disk, gone.0).is_err(), "the first image's ESP is still in the table"); + drop(disk); + + install(&second, &path, 2 * DATA); + let (remade, start, bytes, stamp) = data_of(&mut open()); + assert_ne!(remade, guid); + assert_eq!((start, bytes), (PARTITION_ALIGN as u64, 2 * DATA)); + assert_eq!(stamp, designation(2 * DATA / SECTOR as u64)); + } + /// **One ordering of one set is one image.** The judge below compares /// `root=` against the superblock the same build stamped, so it is blind to /// this: a `root_uuid` returning a constant satisfies it. This is the arm diff --git a/src/qemu.rs b/src/qemu.rs index 32800acce2a..c29d67fcf51 100644 --- a/src/qemu.rs +++ b/src/qemu.rs @@ -108,6 +108,22 @@ impl Profile { } } +/// Where a machine's image is. +enum Medium { + /// A USB stick on the xHCI, which the kernel drives, and DATA on an NVMe + /// disk of its own, `target/nvme.img`. + Stick, + /// The machine's one NVMe disk, [`DISK`]: the image installed around the + /// DATA that file already holds, so `/home` outlives a rebuild. + Disk, +} + +/// The disk of every machine that boots off one, whichever image it runs. +const DISK: &str = "target/disk.img"; + +/// The DATA partition a [`DISK`] is made with. +const DATA_BYTES: u64 = 1024 * 1024 * 1024; + pub struct Options { pub arch: Arch, pub debug: bool, @@ -125,6 +141,19 @@ pub struct Options { pub image: PathBuf, } +impl Options { + fn medium(&self) -> Medium { + match (self.profile, self.arch) { + // The T14 boots from a stick, and its shape is this profile's. + (Profile::Metal, _) => Medium::Stick, + // No process on `virt` is handed a controller, so a disk there + // would serve nothing its stick does not. + (Profile::Virtio | Profile::Gop, Arch::Aarch64) => Medium::Stick, + (Profile::Virtio | Profile::Gop, Arch::X86_64) => Medium::Disk, + } + } +} + pub fn launch(opts: &Options) { let shape = opts.profile.shape(); let arch = opts.arch; @@ -176,19 +205,29 @@ pub fn launch(opts: &Options) { // the unit never sees it, whatever the tables say. let platform = if shape.iommu { ",iommu_platform=on" } else { "" }; - qemu.arg("-device") - .arg("nec-usb-xhci,id=xhci") - .arg("-drive") - .arg(format!( - "if=none,id=stick,format=raw,file={}", - opts.image.display() - )) - .arg("-device") - .arg("usb-storage,bus=xhci.0,drive=stick,bootindex=0") - .arg("-drive") - .arg("if=none,id=nvme0,format=raw,file=target/nvme.img") - .arg("-device") - .arg("nvme,serial=deadbeef,drive=nvme0,msix-exclusive-bar=on"); + qemu.arg("-device").arg("nec-usb-xhci,id=xhci"); + match opts.medium() { + Medium::Stick => qemu + .arg("-drive") + .arg(format!("if=none,id=stick,format=raw,file={}", opts.image.display())) + .arg("-device") + .arg("usb-storage,bus=xhci.0,drive=stick,bootindex=0") + .arg("-drive") + .arg("if=none,id=nvme0,format=raw,file=target/nvme.img") + .arg("-device") + .arg("nvme,serial=deadbeef,drive=nvme0,msix-exclusive-bar=on"), + Medium::Disk => { + let image = std::fs::read(&opts.image) + .unwrap_or_else(|e| panic!("read {} to install it: {e}", opts.image.display())); + toyos_build::image::install(&image, std::path::Path::new(DISK), DATA_BYTES); + qemu.arg("-drive") + .arg(format!("if=none,id=disk,format=raw,file={DISK}")) + .arg("-device") + .arg("nvme,serial=deadbeef,id=nvme0ctl,msix-exclusive-bar=on") + .arg("-device") + .arg("nvme-ns,drive=disk,bootindex=0,bus=nvme0ctl,logical_block_size=512,physical_block_size=512") + } + }; if shape.usb_hid { qemu.arg("-device") diff --git a/tests/common/qemu.rs b/tests/common/qemu.rs index ffd33add970..69d22342e12 100644 --- a/tests/common/qemu.rs +++ b/tests/common/qemu.rs @@ -685,6 +685,10 @@ pub enum Profile { /// `iommu_platform=on`, and the harness sets that only where a unit exists, /// so the guest's own negotiation comes out the other way here. HeadlessNoIommu, + /// [`Profile::Headless`] with no USB controller: the machine whose NVMe + /// disk is the only storage it has, and the only device its firmware can + /// boot. + HeadlessNoUsb, /// M1 metal-sim: GOP, NVMe, xHCI with the boot stick on it, i8042 from /// q35, and nothing else -- no virtio device and no USB HID. This is the /// machine shape that gets flashed, so it is the one the input tests run @@ -721,6 +725,7 @@ impl Profile { Self::Virt | Self::VirtEl2 | Self::VirtEl2NoVhe | Self::VirtTcg => Arch::Aarch64, Self::Headless | Self::HeadlessNoIommu + | Self::HeadlessNoUsb | Self::Metal => Arch::X86_64, } } @@ -830,17 +835,38 @@ struct Shape { /// one input handler per device class, so with a usb-kbd present every /// injected keystroke goes to it. usb: &'static [&'static str], - /// The NVMe namespace's size. The backing file is sparse, so this is free - /// to state honestly — and it has to be stated, because a kernel - /// structure sized per device block is bounded by this number and by - /// nothing else. - nvme_bytes: u64, + storage: Storage, /// The unit that decodes this machine's DMA, or its absence. Stated per /// profile because absence is a shape and because the unit's own /// capabilities are what the kernel reads at boot. iommu: Option, } +/// Where a machine's image and its DATA are. A size is stated because a +/// structure sized per device block is bounded by it and by nothing else; the +/// backing file is sparse either way. +#[derive(Clone, Copy, PartialEq, Eq)] +enum Storage { + /// The image on a USB stick on the first xHCI, which the kernel drives, + /// and DATA on an NVMe disk of its own of this many bytes; zero is a + /// machine with no NVMe controller. + Stick { nvme_bytes: u64 }, + /// One NVMe disk and no stick: DATA, of this many bytes, and the image + /// installed after it (`toyos_build::image::install`). The firmware boots + /// it, and after the loader every partition of it is `diskserver`'s. + Disk { data_bytes: u64 }, +} + +impl Storage { + /// The id of the `-drive` the image's file is. + fn image_drive(self) -> &'static str { + match self { + Self::Stick { .. } => "stick", + Self::Disk { .. } => "disk", + } + } +} + /// The boot stick's device id: the removal the owner's machine dies on is the /// one device whose removal takes `/boot` and `/log` with it. pub const BOOT_STICK_ID: &str = "bootstick"; @@ -851,8 +877,8 @@ pub const BOOT_STICK_ID: &str = "bootstick"; /// what a test moving it has to be able to say is not so. pub const BOOT_STICK_SERIAL: &str = "TOYOS0BOOTSTICK1"; -/// What every x86-64 profile gives the guest. Large enough for a filesystem, -/// small enough that a boot formats it quickly. +/// The DATA every x86-64 profile gives the guest. Large enough for a +/// filesystem, small enough that a boot formats it quickly. pub const NVME_SMALL: u64 = 128 * 1024 * 1024; impl Profile { @@ -866,7 +892,7 @@ impl Profile { nic: Nic::Absent, xhci: &[XHCI_DEFAULT], usb: &[], - nvme_bytes: 0, + storage: Storage::Stick { nvme_bytes: 0 }, iommu: None, }, Self::Headless => Shape { @@ -876,7 +902,7 @@ impl Profile { nic: Nic::Virtio, xhci: &[XHCI_DEFAULT], usb: &["usb-kbd,bus=xhci.0"], - nvme_bytes: NVME_SMALL, + storage: Storage::Disk { data_bytes: NVME_SMALL }, iommu: Some(IOMMU_DEFAULT), }, Self::Metal => Shape { @@ -890,10 +916,11 @@ impl Profile { nic: Nic::Absent, xhci: &[XHCI_DEFAULT], usb: &[], - nvme_bytes: NVME_SMALL, + storage: Storage::Stick { nvme_bytes: NVME_SMALL }, iommu: Some(IOMMU_DEFAULT), }, Self::HeadlessNoIommu => Shape { iommu: None, ..Self::Headless.shape() }, + Self::HeadlessNoUsb => Shape { xhci: &[], usb: &[], ..Self::Headless.shape() }, } } @@ -1020,6 +1047,10 @@ pub struct QemuInstance { smp: u32, /// The test binaries this boot put on ROOT, by the name `run` takes. carried: BTreeSet, + /// What this machine was booted with, and the 16550's file: what a + /// second wait for its ready marker reads. + options: BootOptions, + uart_log: PathBuf, } /// Which of [`DECLARED_KERNEL_BUILDS`] this boot wants. @@ -1205,7 +1236,11 @@ impl QemuInstance { ¶ms, options.debug_wait, ); - fs::write(&boot_image, image).expect("Failed to write test boot image"); + let storage = options.profile.shape().storage; + match storage { + Storage::Stick { .. } => fs::write(&boot_image, image).expect("Failed to write test boot image"), + Storage::Disk { data_bytes } => toyos_build::image::install(&image, &boot_image, data_bytes), + } let carried = c_tests .iter() .map(|(name, _)| format!("test_c_{name}")) @@ -1219,16 +1254,20 @@ impl QemuInstance { // **Every boot gets a blank DATA volume**, so what one boot leaves under // `/home` — sshserver's host identity, a package, a cache — is never the - // premise of whatever test the lane runs next. The lane's one file is - // remade rather than a file per boot. + // premise of whatever test the lane runs next. A disk that carries the + // image is its boot's own file, made just above; a DATA disk beside a + // stick is the lane's one file, remade rather than a file per boot. // // One live guest per image, claimed here rather than discovered from // QEMU's stderr after the second process has already exited — see // [`NvmeClaim`] — and claimed before the remaking, which truncates. - let nvme_bytes = options.profile.shape().nvme_bytes; + let nvme_bytes = match storage { + Storage::Stick { nvme_bytes } => nvme_bytes, + Storage::Disk { .. } => 0, + }; let nvme_image = if nvme_bytes == 0 { - // A profile with no controller gets no backing file either; the - // path is never passed to QEMU. + // A profile with no NVMe disk beside its image gets no backing + // file either; the path is never passed to QEMU. test_dir.join("no-nvme") } else { test_dir.join(format!("test-nvme-{nvme_bytes}.img")) @@ -1258,7 +1297,7 @@ impl QemuInstance { let qemu = qemu_command(&boot_image, nvme.path(), &uart_log, &sockets.dir, &vars, &options); spawn_and_wait_ready( qemu, - &options, + options, Files { seq, uart_log, @@ -1272,6 +1311,20 @@ impl QemuInstance { ) } + /// From here a reset this guest asks for resets the machine, its memory + /// and its disks kept, where `-no-reboot` would have ended QEMU with it. + pub fn reset_on_reboot(&mut self) { + let socket = self.sockets.qmp.clone().expect("reset_on_reboot needs BootOptions { qmp: true }"); + Qmp::connect(&socket).execute("{\"execute\":\"set-action\",\"arguments\":{\"reboot\":\"reset\"}}"); + } + + /// Wait for this machine to reach its ready marker again, on the boot + /// after a reset: [`Self::boot_log`] is that boot's from here, opening + /// with what the boot before it said last. + pub fn await_boot(&mut self) { + self.boot_log = wait_for_ready(&mut self.child, &self.rx, &self.options, &self.uart_log); + } + /// A span of the guest's *physical* memory, as QEMU reads it. /// /// **The oracle for anything a guest leaves in DRAM for a later boot.** The @@ -2013,8 +2066,12 @@ fn qemu_command( .arg("-drive") .arg(firmware_vars) .arg("-drive") - .arg(format!("if=none,id=stick,format=raw,file={}", boot_image.display())); + .arg(format!("if=none,id={},format=raw,file={}", shape.storage.image_drive(), boot_image.display())); assert!(!shape.xhci.is_empty() || shape.usb.is_empty(), "a USB device needs a controller"); + assert!( + !shape.xhci.is_empty() || matches!(shape.storage, Storage::Disk { .. }), + "a boot stick needs a controller" + ); // Ahead of every other `-device`: QEMU gives a PCI function the bypassing // address space unless the unit exists when the function is created, so a @@ -2037,9 +2094,11 @@ fn qemu_command( qemu.arg("-device").arg(*controller); } - qemu.arg("-device").arg(format!( - "usb-storage,bus=xhci.0,drive=stick,id={BOOT_STICK_ID},serial={BOOT_STICK_SERIAL},bootindex=0" - )); + if let Storage::Stick { .. } = shape.storage { + qemu.arg("-device").arg(format!( + "usb-storage,bus=xhci.0,drive=stick,id={BOOT_STICK_ID},serial={BOOT_STICK_SERIAL},bootindex=0" + )); + } match (arch, shape.vga) { (Arch::X86_64, vga) => { qemu.arg("-vga").arg(vga); @@ -2076,17 +2135,22 @@ fn qemu_command( // no console line and no screendump can see a device that is absent. // // Its MSI-X table in a BAR of its own, because diskserver drives it and a claim - // never maps the BAR holding the table. On a machine that boots off NVMe it - // answers under Intel's ids, so the `pci:1b36:0010` row names the boot - // controller alone and this one is nobody's, as the kernel's first-by-class - // probe left it. - if shape.nvme_bytes != 0 { - qemu.arg("-drive") - .arg(format!("if=none,id=nvme0,format=raw,file={}", nvme_image.display())) - .arg("-device") + // never maps the BAR holding the table. One controller and one namespace + // whichever disk it is: diskserver reads namespace 1 of the one controller + // its row names. + let namespace = match shape.storage { + Storage::Stick { nvme_bytes: 0 } => None, + Storage::Stick { .. } => { + qemu.arg("-drive").arg(format!("if=none,id=nvme0,format=raw,file={}", nvme_image.display())); + Some("drive=nvme0") + } + Storage::Disk { .. } => Some("drive=disk,bootindex=0"), + }; + if let Some(namespace) = namespace { + qemu.arg("-device") .arg("nvme,serial=deadbeef,id=nvme0ctl,msix-exclusive-bar=on") .arg("-device") - .arg("nvme-ns,drive=nvme0,bus=nvme0ctl,logical_block_size=512,physical_block_size=512"); + .arg(format!("nvme-ns,{namespace},bus=nvme0ctl,logical_block_size=512,physical_block_size=512")); } for dev in shape.usb { qemu.arg("-device").arg(*dev); @@ -2191,7 +2255,7 @@ struct Files { carried: BTreeSet, } -fn spawn_and_wait_ready(mut qemu: Command, options: &BootOptions, files: Files) -> QemuInstance { +fn spawn_and_wait_ready(mut qemu: Command, options: BootOptions, files: Files) -> QemuInstance { let Files { seq, uart_log, nvme, sockets, screendump, boot_image, vars, carried } = files; // Inherited: `orphan` reads QEMU's exit as the end of its harness's stderr. @@ -2256,7 +2320,7 @@ fn spawn_and_wait_ready(mut qemu: Command, options: &BootOptions, files: Files) let boot_log = if options.mute { String::new() } else { - wait_for_ready(&mut child, &rx, options, &uart_log) + wait_for_ready(&mut child, &rx, &options, &uart_log) }; // The ready marker is test-runner's, which the supervisor starts after its // first line, so a boot that reached it has said its build, and the kernel @@ -2299,6 +2363,8 @@ fn spawn_and_wait_ready(mut qemu: Command, options: &BootOptions, files: Files) boot_log, smp: options.smp, carried, + options, + uart_log, } } diff --git a/tests/toyos.rs b/tests/toyos.rs index 7db775abb22..eaa2bb40fb2 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -307,6 +307,12 @@ const MACHINE_TESTS: &[&str] = &[ // `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", + // A machine whose one NVMe disk is all it has: its firmware boots the + // image off it, and `/log` and `/home` are `diskserver`'s partitions of + // it across a reset. The claim on the controller and the firmware that + // reads it have no host build, and the T14 boots from a stick beside an + // NVMe disk that is another system's. + "nvme_disk_keeps_log_and_home", ]; /// **The metal profile**: which registrations run on the ThinkPad T14, what @@ -3106,10 +3112,121 @@ fn run_machine_test(name: &str, test_config: &Path) -> Result<(), String> { "machine_shutdown_short_stop" => power::machine_shutdown_short_stop(test_config), "bar_map_again" => bar_map_again(test_config), "console_image_boots" => console_image_boots(), + "nvme_disk_keeps_log_and_home" => nvme_disk_keeps_log_and_home(test_config), other => Err(format!("unknown machine test {other}")), } } +/// What each role's file server says once it serves a volume it mounted, up to +/// the volume's own counts. +const SERVED_FROM_DISK: [&str; 3] = [ + "fileserver: Data serving /apps, /config, /home, /state — bcachefs, ", + "fileserver: Log serving /log — FAT32, ", + "fileserver: Boot serving /boot — FAT32 read-only, ", +]; + +/// Wait for the three file servers of a machine with no disk the kernel +/// drives to say what they serve: each a volume it mounted, on a session of +/// its own that `diskserver` opened, and ROOT's partition refused to all. +fn served_by_diskserver(qemu: &mut QemuInstance, console: &mut String) -> Result<(), String> { + await_guest(qemu, console, "the three file servers to say what they serve", |c| { + ["Data", "Log", "Boot"].iter().all(|role| c.contains(&format!("fileserver: {role} serving "))) + }) + .map_err(|e| format!("{e}\n{console}"))?; + let said = serial::Serial::named("the boot", console.clone()); + for served in SERVED_FROM_DISK { + said.must_say(served)?; + } + said.must_say("is the ROOT this machine runs from; refusing every session to it")?; + let sessions: BTreeSet<&str> = said + .text() + .lines() + .filter(|line| line.contains("diskserver: session ")) + .filter_map(|line| line.split_once(" opened ")?.1.split_whitespace().next()) + .collect(); + if sessions.len() != SERVED_FROM_DISK.len() { + return Err(format!( + "diskserver opened sessions on {sessions:?}, and DATA, the log and the slot's volume are three partitions\n{console}" + )); + } + Ok(()) +} + +/// Run `command` as a job of the boot, to exit 0: what it said. +fn job_said(qemu: &mut QemuInstance, command: &str) -> Result { + let result = qemu.run_test(command, Duration::from_secs(60)); + if let Some(why) = &result.error { + return Err(format!("`{command}`: {why}\nit said:\n{}", result.stdout)); + } + if result.exit_code != Some(0) { + return Err(format!("`{command}` ended {:?}:\n{}", result.exit_code, result.stdout)); + } + Ok(result.stdout) +} + +/// A machine with one NVMe disk and no USB controller boots the image off +/// that disk, and what one boot wrote to `/home` and said into `/log` the next +/// boot reads back: a file under a name of this run's, byte for byte, and a +/// line of this run's in a log file the first boot listed. +fn nvme_disk_keeps_log_and_home(test_config: &Path) -> Result<(), String> { + let options = BootOptions { profile: qemu::Profile::HeadlessNoUsb, qmp: true, ..Default::default() }; + if let Some(usb) = qemu::profile_argv(&options).iter().find(|arg| arg.contains("usb") || arg.contains("xhci")) { + return Err(format!("this machine was to have no USB controller, and QEMU is given {usb}")); + } + let since = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).expect("this host's clock is past 1970"); + let nonce = format!("kept-{}-{}", std::process::id(), since.as_nanos()); + let kept = format!("/home/{nonce}/release"); + let source = toyos_osrelease::GUEST_PATH; + + let mut qemu = QemuInstance::boot_with_options(test_config, &[], &[], options); + let mut console = qemu.boot_log().to_string(); + served_by_diskserver(&mut qemu, &mut console)?; + job_said(&mut qemu, &format!("mkdir /home/{nonce}"))?; + job_said(&mut qemu, &format!("cp {source} {kept}"))?; + job_said(&mut qemu, &format!("echo {nonce}"))?; + // `ls` says a file as two spaces, its name, and its size in brackets. + let logs: Vec = job_said(&mut qemu, "ls /log")? + .lines() + .filter_map(|line| Some(line.strip_prefix(" ")?.split_once(" (")?.0.to_string())) + .collect(); + if logs.is_empty() { + return Err(format!("the first boot's /log lists no file\n{console}")); + } + + // The reset the guest asks for, and the one its loader makes of the pass + // that reads how that boot ended: the boot after both is the one waited for. + qemu.reset_on_reboot(); + writeln!(qemu.stdin_mut(), "run reboot").expect("write to QEMU stdin"); + qemu.flush_stdin(); + qemu.await_boot(); + let mut console = qemu.boot_log().to_string(); + if !console.contains(bootlog::REBOOTING) { + return Err(format!("the machine reached {} again without saying {}\n{console}", qemu::DEFAULT_READY, bootlog::REBOOTING)); + } + // What the second boot said, and none of the first's servers' lines. + console = console.split_once(bootlog::REBOOTING).expect("checked above").1.to_string(); + served_by_diskserver(&mut qemu, &mut console)?; + let shipped = job_said(&mut qemu, &format!("cat {source}"))?; + let read_back = job_said(&mut qemu, &format!("cat {kept}"))?; + if shipped.trim().is_empty() || read_back != shipped { + return Err(format!("{kept} reads back after the reboot as\n{read_back}\nand {source} is\n{shipped}")); + } + // The line as the first boot's `logkeeper` filed it: `echo`'s own, whose + // whole text is the nonce, which nothing the second boot says is. + let mut found = None; + for log in &logs { + let lines = job_said(&mut qemu, &format!("grep {nonce} /log/{log}"))?; + found = found.or(lines.lines().find_map(|line| { + toyos_logstream::program_line(line).filter(|said| said.text == nonce).map(|said| format!("{log}: {}", said.tag)) + })); + } + let Some(found) = found else { + return Err(format!("none of {logs:?}, the first boot's log files, holds its line {nonce} after the reboot")); + }; + eprintln!(" [disk] after the reboot {kept} is {} bytes of {source}, and /log/{found} said {nonce}", read_back.len()); + Ok(()) +} + /// A claim's memory BAR asked for again — while an earlier answer is held, and /// once its handle is gone, closed or never installed for want of room: each /// time the kernel answers a handle whose mapping reads the function's own From a3ef55ba1d5ef025f64cfcde020157059e086f79 Mon Sep 17 00:00:00 2001 From: japabu Date: Fri, 9 Oct 2026 12:44:02 +0200 Subject: [PATCH 2/6] The install test writes where the first image's log partition holds nothing, and the second image's bytes are read back there Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A --- src/image.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/image.rs b/src/image.rs index 4a5146b0f4c..fc4560d6e17 100644 --- a/src/image.rs +++ b/src/image.rs @@ -1308,9 +1308,10 @@ mod tests { } /// An install puts a designated DATA first and the image after it; a - /// second install, of an image of another size, replaces the image and - /// leaves DATA where it was, under its GUID, holding what was written to - /// it; and a DATA of another size is a disk made anew. + /// second install, of an image of another size, puts that image's bytes + /// where the first's were written over, and leaves DATA where it was, + /// under its GUID, holding what was written to it; and a DATA of another + /// size is a disk made anew. #[test] fn an_install_replaces_the_image_around_the_data_it_keeps() { const DATA: u64 = 4 << 20; @@ -1328,10 +1329,12 @@ mod tests { assert_eq!((start, bytes), (PARTITION_ALIGN as u64, DATA)); assert_eq!(stamp, designation(DATA / SECTOR as u64)); - // What a volume's format does to the stamp, and a block at DATA's end. + // What a volume's format does to the stamp, a block at DATA's end, and + // what a boot writes where its image's log partition holds nothing. let written = vec![0xA5u8; SECTOR]; - for at in [start, start + bytes - SECTOR as u64] { - disk.seek(SeekFrom::Start(at)).and_then(|_| disk.write_all(&written)).expect("write DATA"); + let (log_at, log_bytes) = partition_extent(&mut disk, only(&first, toyos_gpt::Guid::MICROSOFT_BASIC).0).expect("the log"); + for at in [start, start + bytes - SECTOR as u64, log_at + log_bytes - SECTOR as u64] { + disk.seek(SeekFrom::Start(at)).and_then(|_| disk.write_all(&written)).expect("write the disk"); } drop(disk); From 9b719d01a95c4f6b1c5ffc20f608dae4873b52cb Mon Sep 17 00:00:00 2001 From: japabu Date: Fri, 9 Oct 2026 13:00:38 +0200 Subject: [PATCH 3/6] The slots issue names the machines that boot off a disk now, and the probe issue the Headless reading `Profile::InternalDisk` went with the guest cut; the machines whose slots no claim reaches are every `Storage::Disk` guest and `cargo run`'s x86-64 machine. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A --- ...off-nvme-looks-1500-ms-for-a-usb-boot-disk.md | 6 ++++-- ...k-diskserver-drives-cannot-write-its-slots.md | 16 ++++++++++------ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/issues/a-machine-that-boots-off-nvme-looks-1500-ms-for-a-usb-boot-disk.md b/issues/a-machine-that-boots-off-nvme-looks-1500-ms-for-a-usb-boot-disk.md index b53c4eac644..33ec63fc0ec 100644 --- a/issues/a-machine-that-boots-off-nvme-looks-1500-ms-for-a-usb-boot-disk.md +++ b/issues/a-machine-that-boots-off-nvme-looks-1500-ms-for-a-usb-boot-disk.md @@ -18,8 +18,10 @@ controller at all, on both of its boots: the kernel says `xHCI: no controller on this machine, USB input unavailable` and, stamped 1501 ms later (`2.917` and `4.418` on the test's first run), `usb-storage: 0 disk(s) on this machine and none carries the boot partition after 1500 ms of looking`. Those boots -report `kernel to Boot: complete 1741 ms`. Every machine that boots off its -NVMe disk pays it, `cargo run`'s among them. +report `kernel to Boot: complete 1741 ms`. And on `bar_map_again`'s, a +`Headless` machine whose xHCI carries a keyboard and no disk, which is the +shape `cargo run` boots on x86-64: `usb-storage: 0 device(s)` at `6.126` and +the same line at `7.628`. The kernel already knows the outcome it waits for is impossible on a machine with no xHCI controller, and it is told nothing about which bus the boot diff --git a/issues/an-image-on-a-disk-diskserver-drives-cannot-write-its-slots.md b/issues/an-image-on-a-disk-diskserver-drives-cannot-write-its-slots.md index 1c9622edcb4..d51e30a5172 100644 --- a/issues/an-image-on-a-disk-diskserver-drives-cannot-write-its-slots.md +++ b/issues/an-image-on-a-disk-diskserver-drives-cannot-write-its-slots.md @@ -11,12 +11,16 @@ The updater writes the idle slot through a partition claim `slots` row), and the kernel answers a claim only on the disks it drives — the USB disks, until usbd drives the controller. The kernel drives no NVMe controller: blockd does (`userland/blockd`). So a machine booted from an image -on its NVMe disk — `Profile::InternalDisk`, and the T14 once ToyOS is installed -on its internal disk — finds its slots nowhere a claim reaches, and an update -is refused at the claim. - -Every update test boots off a USB stick, which is why nothing fails. +on its NVMe disk — every guest whose profile's storage is `Storage::Disk` +(`tests/common/qemu.rs`), the x86-64 machine `cargo run` boots +(`src/qemu.rs`'s `Medium::Disk`), and the T14 once ToyOS is installed on its +internal disk — finds its slots nowhere a claim reaches, and an update is +refused at the claim: `slot_grant` (`userland/supervisor/src/main.rs`) asks +the inventory for the ROOT partition the kernel holds, and the kernel lists +no partition of a disk it does not drive. Read off that function, and not +run: the tree holds no test that runs an update on any machine, which is why +nothing fails. **Exit**: the slots of an image on a disk blockd drives are written through a session init grants the updater on blockd's port, judged by an update test -that boots `Profile::InternalDisk`. +that boots a profile whose storage is `Storage::Disk`. From a4033a5d26b207dcec81a0d887d8862bed273588 Mon Sep 17 00:00:00 2001 From: japabu Date: Fri, 9 Oct 2026 13:22:50 +0200 Subject: [PATCH 4/6] The launcher stays on the stick, and an install makes its disk or refuses the path Review round 1 of the image on NVMe. `cargo run` boots what it booted on main: `Medium`, `DISK`, `DATA_BYTES` and the launcher's disk arm are gone, and `src/qemu.rs` is main's file again. A machine that updates itself keeps its image where a partition claim reaches its slots; the slots issue is the exit for moving it, says `diskserver` where it said `blockd`, names the supervisor where it said init, and no longer names `cargo run` as a machine that lost its update. `image::install` had a keep-DATA arm whose only caller was that launcher, and whose "not the disk I made" answer was to truncate. Both go: `data_on`, the `kept` branch, the cut-and-grow and the megabyte clear. An install is DATA, then the image, on a file it creates with `create_new`, and a path that names anything already is refused by name. The host test is the install's layout and that refusal, with the refused file read back unchanged. The stage that boots a machine a user keeps a `/home` on brings a keep arm back with its refusal. Issues: the thirty seconds an image with `fileserver` and no `diskserver` stands still on a disk machine, found by a mutation of this branch's test, is filed; the no-IOMMU issue says that on a disk machine the refused controller takes LOG and BOOT with DATA, as the no-unit arm of `iommu_virtio_platform` reads at this tree; the probe issue stops calling `Headless` the launcher's shape. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A --- ...ock-port-nobody-serves-and-says-nothing.md | 47 +++++++ ...-nvme-looks-1500-ms-for-a-usb-boot-disk.md | 5 +- ...ne-without-an-iommu-refuses-every-claim.md | 12 ++ ...iskserver-drives-cannot-write-its-slots.md | 40 +++--- src/image.rs | 120 +++++------------- src/qemu.rs | 65 ++-------- 6 files changed, 130 insertions(+), 159 deletions(-) create mode 100644 issues/a-file-server-waits-30-s-on-a-block-port-nobody-serves-and-says-nothing.md diff --git a/issues/a-file-server-waits-30-s-on-a-block-port-nobody-serves-and-says-nothing.md b/issues/a-file-server-waits-30-s-on-a-block-port-nobody-serves-and-says-nothing.md new file mode 100644 index 00000000000..7f09d251763 --- /dev/null +++ b/issues/a-file-server-waits-30-s-on-a-block-port-nobody-serves-and-says-nothing.md @@ -0,0 +1,47 @@ +--- +status: open +kind: defect +opened: 2026-10-09 +--- + +# A file server waits 30 s on a block port nobody serves, and says nothing + +An image whose `[boot]` `start` runs `fileserver` and no `diskserver`, on a +machine whose every partition is on a disk the kernel does not drive, stands +still for about thirty seconds after its three file servers are spawned, and +no line of the boot says who is waiting or on what. + +Measured once, on `nvme_disk_keeps_log_and_home`'s machine with `diskserver` +taken out of `tests/testcases/system.toml`'s `start` (host load 52 by +`uptime`'s one-minute mean); the guest's own stamps: + +``` +[10.330 cpu0 kernel] spawn: /system/bin/fileserver pid=2 … +[10.362 cpu0 kernel] spawn: /system/bin/fileserver pid=3 … +[10.377 cpu0 kernel] spawn: /system/bin/fileserver pid=4 … +[40.513 cpu0 kernel] spawn: /system/bin/logkeeper pid=5 … +[40.526 cpu0 kernel] spawn: /system/bin/soundserver pid=6 … +``` + +No `fileserver:`, `supervisor:` or kernel line falls between the third and +the fourth, and none after it names a wait that ran out. The test went red +only on its own ceiling, `[qemu] Boot timed out waiting for ===READY===`. + +Not read: which call waits, and whose bound the thirty seconds are. +`fileserver` reaches the port through `diskserver::list` and +`diskserver::Session::open` on `toyos_blockring::PORT` +(`userland/fileserver/src/main.rs`), and the supervisor starts `logkeeper` +only after the file servers. + +Where `diskserver` runs and is refused its controller the same servers answer +at once and by name (`fileserver: Log serving /log — absent: …`), so the wait +is the unserved port's alone. + +**Exit condition.** On that boot the file servers say, by role, that no block +service answers, and the boot reaches its ready marker without the wait: a +guest test of an image with `fileserver` and no `diskserver` on a +`Storage::Disk` profile requires the three lines and reds on the gap. + +## Owner + +`userland/fileserver` and `userland/supervisor`; unheld. diff --git a/issues/a-machine-that-boots-off-nvme-looks-1500-ms-for-a-usb-boot-disk.md b/issues/a-machine-that-boots-off-nvme-looks-1500-ms-for-a-usb-boot-disk.md index 33ec63fc0ec..ba8b2790220 100644 --- a/issues/a-machine-that-boots-off-nvme-looks-1500-ms-for-a-usb-boot-disk.md +++ b/issues/a-machine-that-boots-off-nvme-looks-1500-ms-for-a-usb-boot-disk.md @@ -19,9 +19,8 @@ on this machine, USB input unavailable` and, stamped 1501 ms later (`2.917` and `4.418` on the test's first run), `usb-storage: 0 disk(s) on this machine and none carries the boot partition after 1500 ms of looking`. Those boots report `kernel to Boot: complete 1741 ms`. And on `bar_map_again`'s, a -`Headless` machine whose xHCI carries a keyboard and no disk, which is the -shape `cargo run` boots on x86-64: `usb-storage: 0 device(s)` at `6.126` and -the same line at `7.628`. +`Headless` machine whose xHCI carries a keyboard and no disk: `usb-storage: 0 +device(s)` at `6.126` and the same line at `7.628`. The kernel already knows the outcome it waits for is impossible on a machine with no xHCI controller, and it is told nothing about which bus the boot diff --git a/issues/a-machine-without-an-iommu-refuses-every-claim.md b/issues/a-machine-without-an-iommu-refuses-every-claim.md index 5f67cab685a..1f9ba8f35fb 100644 --- a/issues/a-machine-without-an-iommu-refuses-every-claim.md +++ b/issues/a-machine-without-an-iommu-refuses-every-claim.md @@ -30,6 +30,18 @@ there is no physical address for a grant to answer with. The signed driver's claim is the path where `NotRemapped` and `Untranslated` both give way under the ruling. +On such a machine whose image is on its NVMe disk, which +`Profile::HeadlessNoIommu` is, the refusal takes every volume but ROOT with +it: `diskserver` is refused the controller (`diskserver: NOT SERVING — +pci:1b36:0010 is on this machine and the kernel refused this service its +claim`), so the three file servers say `Data serving … — absent`, `Log serving +/log — absent` and `Boot serving /boot — absent`, and `logkeeper` says `no +/log on this machine - this boot's kernel log is on the console only`. Read off the no-unit arm of `iommu_virtio_platform`, which is +green with it: the arm asserts the refusal and reads no volume. Where the +image is on a USB stick the kernel drives, LOG and BOOT are served through +kernel partition claims, which need no unit, and DATA alone is absent; that +is read off the claim path and not run. + Exit, in both arms of `iommu_virtio_platform` (`tests/common/iommu.rs`): - On `Profile::HeadlessNoIommu` a claim by a driver the image ships and signs diff --git a/issues/an-image-on-a-disk-diskserver-drives-cannot-write-its-slots.md b/issues/an-image-on-a-disk-diskserver-drives-cannot-write-its-slots.md index d51e30a5172..0224d7d2dc7 100644 --- a/issues/an-image-on-a-disk-diskserver-drives-cannot-write-its-slots.md +++ b/issues/an-image-on-a-disk-diskserver-drives-cannot-write-its-slots.md @@ -4,23 +4,31 @@ kind: defect opened: 2026-09-27 --- -# An image on a disk blockd drives cannot write its slots +# An image on a disk diskserver drives cannot write its slots The updater writes the idle slot through a partition claim (`SYS_DEVICE_CLAIM` with `DeviceType::Partition`, granted by `system.toml`'s -`slots` row), and the kernel answers a claim only on the disks it drives — -the USB disks, until usbd drives the controller. The kernel drives no NVMe -controller: blockd does (`userland/blockd`). So a machine booted from an image -on its NVMe disk — every guest whose profile's storage is `Storage::Disk` -(`tests/common/qemu.rs`), the x86-64 machine `cargo run` boots -(`src/qemu.rs`'s `Medium::Disk`), and the T14 once ToyOS is installed on its -internal disk — finds its slots nowhere a claim reaches, and an update is -refused at the claim: `slot_grant` (`userland/supervisor/src/main.rs`) asks -the inventory for the ROOT partition the kernel holds, and the kernel lists -no partition of a disk it does not drive. Read off that function, and not -run: the tree holds no test that runs an update on any machine, which is why -nothing fails. +`slots` row), and the kernel answers a claim only on the disks it drives: the +USB disks. The kernel drives no NVMe controller: `diskserver` does +(`userland/diskserver`). So a machine booted from an image on its NVMe disk — +every guest whose profile's storage is `Storage::Disk` +(`tests/common/qemu.rs`), and the T14 once ToyOS is installed on its internal +disk — finds its slots nowhere a claim reaches, and an update is refused at +the claim: `slot_grant` (`userland/supervisor/src/main.rs`) asks the inventory +for the ROOT partition the kernel holds, and the kernel lists no partition of +a disk it does not drive. Read off that function, and not run: the tree holds +no test that runs an update on any machine, which is why nothing fails. -**Exit**: the slots of an image on a disk blockd drives are written through a -session init grants the updater on blockd's port, judged by an update test -that boots a profile whose storage is `Storage::Disk`. +The launcher (`src/qemu.rs`) boots every machine it starts off a USB stick +for this reason: a machine that updates itself keeps its image where a claim +reaches its slots, and moves onto its NVMe disk only once this issue's exit +is met. + +**Exit**: the slots of an image on a disk `diskserver` drives are written +through a session the supervisor grants the updater on `diskserver`'s port, +judged by an update test that boots a profile whose storage is +`Storage::Disk`. + +## Owner + +`userland/supervisor` and `userland/diskserver`; unheld. diff --git a/src/image.rs b/src/image.rs index fc4560d6e17..7b28e9ad0fa 100644 --- a/src/image.rs +++ b/src/image.rs @@ -724,11 +724,11 @@ fn designate(device: &mut gpt::DiskDeviceObject<'_>, at: u64, bytes: u64) { /// on the file at `disk`: DATA, `data_bytes` of it, and after it every /// partition of the image under the name, type and GUID the image gave it. /// -/// **A disk that already holds that DATA partition keeps it**, its bytes and -/// its GUID, and the image is replaced around it; any other file is made anew -/// and its DATA designated. DATA comes first so that no image's size moves it. -/// Only the tables, the stamp and the image's blocks that are not zero are -/// written, so the file costs the host what the image does. +/// **The file is one this call makes**: a path that already names anything is +/// refused, so nothing that was there is written over. DATA comes first so +/// that no image's size moves it. Only the tables, the stamp and the image's +/// blocks that are not zero are written, so the file costs the host what the +/// image does. pub fn install(image: &[u8], disk: &Path, data_bytes: u64) { use gpt::disk::LogicalBlockSize::Lb512; let mut listed = [None; 16]; @@ -759,31 +759,20 @@ pub fn install(image: &[u8], disk: &Path, data_bytes: u64) { }) .collect(); - let kept = data_on(disk, data_bytes); let (mut rows, volumes): (Vec, Vec<&[u8]>) = carried.into_iter().unzip(); - rows.insert(0, data_row(kept.unwrap_or_else(uuid::Uuid::new_v4), data_bytes)); + rows.insert(0, data_row(uuid::Uuid::new_v4(), data_bytes)); let total = disk_bytes(rows.iter().map(|row| row.len)); - let mut file = std::fs::OpenOptions::new() + let file = std::fs::OpenOptions::new() .read(true) .write(true) - .create(true) - .truncate(kept.is_none()) + .create_new(true) .open(disk) - .unwrap_or_else(|e| panic!("open {} to install on it: {e}", disk.display())); - // Cut to DATA's end and grown again, with the old table's megabyte - // cleared: everything but DATA reads zero from here. - let data_end = PARTITION_ALIGN as u64 + data_bytes; - file.set_len(data_end) - .and_then(|()| file.set_len(total)) - .and_then(|()| file.write_all(&vec![0u8; PARTITION_ALIGN])) - .unwrap_or_else(|e| panic!("clear {} around its DATA: {e}", disk.display())); + .unwrap_or_else(|e| panic!("{} is not installed on, being no file this install made: {e}", disk.display())); + file.set_len(total).unwrap_or_else(|e| panic!("size {} to {total} bytes: {e}", disk.display())); let (mut device, starts) = lay(Box::new(file), total, &rows); - assert_eq!(starts[0], PARTITION_ALIGN as u64, "DATA is not the disk's first partition"); - if kept.is_none() { - designate(&mut device, starts[0], data_bytes); - } + designate(&mut device, starts[0], data_bytes); for ((row, volume), start) in rows[1..].iter().zip(volumes).zip(&starts[1..]) { for (i, block) in volume.chunks(PARTITION_ALIGN).enumerate().filter(|(_, block)| block.iter().any(|byte| *byte != 0)) { let at = start + (i * PARTITION_ALIGN) as u64; @@ -796,15 +785,6 @@ pub fn install(image: &[u8], disk: &Path, data_bytes: u64) { device.flush().unwrap_or_else(|e| panic!("flush {}: {e}", disk.display())); } -/// The unique GUID of the DATA partition [`install`] put on the file at -/// `disk`, where it holds one of `data_bytes`: the one an install keeps. -fn data_on(disk: &Path, data_bytes: u64) -> Option { - let mut file = std::fs::File::open(disk).ok()?; - let part = only_partition(&mut FileSectors(&mut file), toyos_gpt::Guid::TOYOS_DATA).ok()?; - let (start, len) = (part.first_lba() * u64::from(LBA), part.lba_count().get() * u64::from(LBA)); - (start == PARTITION_ALIGN as u64 && len == data_bytes).then(|| uuid::Uuid::from_bytes_le(part.unique_guid().0)) -} - /// Block 0 of a volume: the magic and its block count. fn designation(blocks: u64) -> [u8; SECTOR] { let mut block = [0u8; SECTOR]; @@ -1298,72 +1278,36 @@ mod tests { } } - /// The one DATA partition on `disk`: its GUID, its extent and its first block. - fn data_of(disk: &mut std::fs::File) -> (toyos_gpt::Guid, u64, u64, Vec) { - let part = only_partition(&mut FileSectors(disk), toyos_gpt::Guid::TOYOS_DATA).expect("one DATA partition"); - let (start, bytes) = partition_extent(disk, part.unique_guid().0).expect("DATA's extent"); - let mut first = vec![0u8; SECTOR]; - disk.seek(SeekFrom::Start(start)).and_then(|_| disk.read_exact(&mut first)).expect("read DATA"); - (part.unique_guid(), start, bytes, first) - } - - /// An install puts a designated DATA first and the image after it; a - /// second install, of an image of another size, puts that image's bytes - /// where the first's were written over, and leaves DATA where it was, - /// under its GUID, holding what was written to it; and a DATA of another - /// size is a disk made anew. + /// An install puts a designated DATA first and the image after it, on a + /// file it makes; a path that names a file already is refused, and that + /// file is left as it was. #[test] - fn an_install_replaces_the_image_around_the_data_it_keeps() { + fn an_install_makes_its_disk_and_refuses_one_that_exists() { const DATA: u64 = 4 << 20; let key = key(); - let root = tiny_root(); + let image = create_boot_image(Arch::X86_64, b"kernel", b"bootloader", &tiny_root(), "", signing(&key), None); let dir = toyos_tmpdir::TempDir::new("image-install"); let path = dir.join("disk.img"); - let open = || std::fs::OpenOptions::new().read(true).write(true).open(&path).expect("open the disk"); - - let first = create_boot_image(Arch::X86_64, b"kernel", b"bootloader", &root, "", signing(&key), None); - install(&first, &path, DATA); - let mut disk = open(); - carries_every_partition_of(&mut disk, &first); - let (guid, start, bytes, stamp) = data_of(&mut disk); - assert_eq!((start, bytes), (PARTITION_ALIGN as u64, DATA)); - assert_eq!(stamp, designation(DATA / SECTOR as u64)); - // What a volume's format does to the stamp, a block at DATA's end, and - // what a boot writes where its image's log partition holds nothing. - let written = vec![0xA5u8; SECTOR]; - let (log_at, log_bytes) = partition_extent(&mut disk, only(&first, toyos_gpt::Guid::MICROSOFT_BASIC).0).expect("the log"); - for at in [start, start + bytes - SECTOR as u64, log_at + log_bytes - SECTOR as u64] { - disk.seek(SeekFrom::Start(at)).and_then(|_| disk.write_all(&written)).expect("write the disk"); - } - drop(disk); - - let second = create_boot_image( - Arch::X86_64, - b"a longer kernel", - b"bootloader", - &root, - "", - signing(&key), - Some(SecondSlot { root_bytes: 8 << 20 }), + install(&image, &path, DATA); + let mut disk = std::fs::File::open(&path).expect("open the disk"); + carries_every_partition_of(&mut disk, &image); + let data = only_partition(&mut FileSectors(&mut disk), toyos_gpt::Guid::TOYOS_DATA).expect("one DATA partition"); + assert_eq!( + (data.first_lba() * u64::from(LBA), data.lba_count().get() * u64::from(LBA)), + (PARTITION_ALIGN as u64, DATA) ); - assert_ne!(first.len(), second.len()); - install(&second, &path, DATA); - let mut disk = open(); - carries_every_partition_of(&mut disk, &second); - assert_eq!(data_of(&mut disk), (guid, start, bytes, written.clone())); - let mut last = vec![0u8; SECTOR]; - disk.seek(SeekFrom::Start(start + bytes - SECTOR as u64)).and_then(|_| disk.read_exact(&mut last)).expect("read DATA"); - assert_eq!(last, written); - let gone = only(&first, toyos_gpt::Guid::EFI_SYSTEM); - assert!(partition_extent(&mut disk, gone.0).is_err(), "the first image's ESP is still in the table"); + let mut stamp = [0u8; SECTOR]; + disk.seek(SeekFrom::Start(PARTITION_ALIGN as u64)).and_then(|_| disk.read_exact(&mut stamp)).expect("read DATA"); + assert_eq!(stamp, designation(DATA / SECTOR as u64)); drop(disk); - install(&second, &path, 2 * DATA); - let (remade, start, bytes, stamp) = data_of(&mut open()); - assert_ne!(remade, guid); - assert_eq!((start, bytes), (PARTITION_ALIGN as u64, 2 * DATA)); - assert_eq!(stamp, designation(2 * DATA / SECTOR as u64)); + let theirs = dir.join("theirs.img"); + std::fs::write(&theirs, b"somebody's").expect("write a file"); + let refused = std::panic::catch_unwind(|| install(&image, &theirs, DATA)).expect_err("an install on a file that exists"); + let said = refused.downcast_ref::().expect("a refusal in words"); + assert!(said.contains("theirs.img"), "the refusal names no path: {said}"); + assert_eq!(std::fs::read(&theirs).expect("read it back"), b"somebody's"); } /// **One ordering of one set is one image.** The judge below compares diff --git a/src/qemu.rs b/src/qemu.rs index c29d67fcf51..32800acce2a 100644 --- a/src/qemu.rs +++ b/src/qemu.rs @@ -108,22 +108,6 @@ impl Profile { } } -/// Where a machine's image is. -enum Medium { - /// A USB stick on the xHCI, which the kernel drives, and DATA on an NVMe - /// disk of its own, `target/nvme.img`. - Stick, - /// The machine's one NVMe disk, [`DISK`]: the image installed around the - /// DATA that file already holds, so `/home` outlives a rebuild. - Disk, -} - -/// The disk of every machine that boots off one, whichever image it runs. -const DISK: &str = "target/disk.img"; - -/// The DATA partition a [`DISK`] is made with. -const DATA_BYTES: u64 = 1024 * 1024 * 1024; - pub struct Options { pub arch: Arch, pub debug: bool, @@ -141,19 +125,6 @@ pub struct Options { pub image: PathBuf, } -impl Options { - fn medium(&self) -> Medium { - match (self.profile, self.arch) { - // The T14 boots from a stick, and its shape is this profile's. - (Profile::Metal, _) => Medium::Stick, - // No process on `virt` is handed a controller, so a disk there - // would serve nothing its stick does not. - (Profile::Virtio | Profile::Gop, Arch::Aarch64) => Medium::Stick, - (Profile::Virtio | Profile::Gop, Arch::X86_64) => Medium::Disk, - } - } -} - pub fn launch(opts: &Options) { let shape = opts.profile.shape(); let arch = opts.arch; @@ -205,29 +176,19 @@ pub fn launch(opts: &Options) { // the unit never sees it, whatever the tables say. let platform = if shape.iommu { ",iommu_platform=on" } else { "" }; - qemu.arg("-device").arg("nec-usb-xhci,id=xhci"); - match opts.medium() { - Medium::Stick => qemu - .arg("-drive") - .arg(format!("if=none,id=stick,format=raw,file={}", opts.image.display())) - .arg("-device") - .arg("usb-storage,bus=xhci.0,drive=stick,bootindex=0") - .arg("-drive") - .arg("if=none,id=nvme0,format=raw,file=target/nvme.img") - .arg("-device") - .arg("nvme,serial=deadbeef,drive=nvme0,msix-exclusive-bar=on"), - Medium::Disk => { - let image = std::fs::read(&opts.image) - .unwrap_or_else(|e| panic!("read {} to install it: {e}", opts.image.display())); - toyos_build::image::install(&image, std::path::Path::new(DISK), DATA_BYTES); - qemu.arg("-drive") - .arg(format!("if=none,id=disk,format=raw,file={DISK}")) - .arg("-device") - .arg("nvme,serial=deadbeef,id=nvme0ctl,msix-exclusive-bar=on") - .arg("-device") - .arg("nvme-ns,drive=disk,bootindex=0,bus=nvme0ctl,logical_block_size=512,physical_block_size=512") - } - }; + qemu.arg("-device") + .arg("nec-usb-xhci,id=xhci") + .arg("-drive") + .arg(format!( + "if=none,id=stick,format=raw,file={}", + opts.image.display() + )) + .arg("-device") + .arg("usb-storage,bus=xhci.0,drive=stick,bootindex=0") + .arg("-drive") + .arg("if=none,id=nvme0,format=raw,file=target/nvme.img") + .arg("-device") + .arg("nvme,serial=deadbeef,drive=nvme0,msix-exclusive-bar=on"); if shape.usb_hid { qemu.arg("-device") From 9a0660e7965e3f6886b6503dd0f56fb56150aa46 Mon Sep 17 00:00:00 2001 From: japabu Date: Fri, 9 Oct 2026 13:42:17 +0200 Subject: [PATCH 5/6] The disk test reads the kept file's own lines out of its job's capture A job's capture is every program's lines of its window. Under the whole suite at a host load of 30 to 48, `acpiserver`'s six boot lines reached the console after the second boot's ready marker, inside `cat /system/etc/os-release`'s window, and the test compared that capture to `cat` of the kept file byte for byte: red, on two files whose ten lines agree. The file's lines are the ones of os-release's form, `KEY=value`, and those are what the two reads are held to. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A --- tests/toyos.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/tests/toyos.rs b/tests/toyos.rs index 45d3e502fe0..6aaebb4e0a0 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -3150,7 +3150,7 @@ fn job_said(qemu: &mut QemuInstance, command: &str) -> Result { /// A machine with one NVMe disk and no USB controller boots the image off /// that disk, and what one boot wrote to `/home` and said into `/log` the next -/// boot reads back: a file under a name of this run's, byte for byte, and a +/// boot reads back: a file under a name of this run's, line for line, and a /// line of this run's in a log file the first boot listed. fn nvme_disk_keeps_log_and_home(test_config: &Path) -> Result<(), String> { let options = BootOptions { profile: qemu::Profile::HeadlessNoUsb, qmp: true, ..Default::default() }; @@ -3190,10 +3190,17 @@ fn nvme_disk_keeps_log_and_home(test_config: &Path) -> Result<(), String> { // What the second boot said, and none of the first's servers' lines. console = console.split_once(bootlog::REBOOTING).expect("checked above").1.to_string(); served_by_diskserver(&mut qemu, &mut console)?; - let shipped = job_said(&mut qemu, &format!("cat {source}"))?; - let read_back = job_said(&mut qemu, &format!("cat {kept}"))?; - if shipped.trim().is_empty() || read_back != shipped { - return Err(format!("{kept} reads back after the reboot as\n{read_back}\nand {source} is\n{shipped}")); + // A job's capture is every program's lines of its window, a server's + // late ones among them: the file's are the ones of its own form, + // `KEY=value`, which no program says. + let assignments = |said: &str| -> Vec { + let key = |line: &str| line.split_once('=').is_some_and(|(key, _)| !key.is_empty() && key.bytes().all(|b| b.is_ascii_uppercase() || b == b'_')); + said.lines().filter(|line| key(line)).map(str::to_string).collect() + }; + let shipped = assignments(&job_said(&mut qemu, &format!("cat {source}"))?); + let read_back = assignments(&job_said(&mut qemu, &format!("cat {kept}"))?); + if shipped.is_empty() || read_back != shipped { + return Err(format!("{kept} reads back after the reboot as\n{read_back:#?}\nand {source} is\n{shipped:#?}")); } // The line as the first boot's `logkeeper` filed it: `echo`'s own, whose // whole text is the nonce, which nothing the second boot says is. @@ -3207,7 +3214,7 @@ fn nvme_disk_keeps_log_and_home(test_config: &Path) -> Result<(), String> { let Some(found) = found else { return Err(format!("none of {logs:?}, the first boot's log files, holds its line {nonce} after the reboot")); }; - eprintln!(" [disk] after the reboot {kept} is {} bytes of {source}, and /log/{found} said {nonce}", read_back.len()); + eprintln!(" [disk] after the reboot {kept} is the {} lines of {source}, and /log/{found} said {nonce}", read_back.len()); Ok(()) } From d55be63bbb757d951ae2a73a44bf23e67ca23dbe Mon Sep 17 00:00:00 2001 From: japabu Date: Fri, 9 Oct 2026 14:01:52 +0200 Subject: [PATCH 6/6] The harness's capture defect is filed, and tests/CLAUDE.md states the fact until it closes A job's `TestResult::stdout` is every program's lines of its window, a server's late boot lines among them: `push_user_half` asks only whether a line is the kernel's. `nvme_disk_keeps_log_and_home` went red on it in the suite at `6b1a9322f` (37 of 38, load 29.6 to 47.9) and has filtered its two reads by the file's line form since `9a0660e79`. The round-2 review of #797 asks for the defect to be filed against `tests/common/qemu.rs` with an exit that deletes the filter, and for the fact as a clause on the two-pieces bullet of `tests/CLAUDE.md`, which goes when the issue closes. No source changes. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A --- ...olds-every-programs-lines-of-its-window.md | 57 +++++++++++++++++++ tests/CLAUDE.md | 2 +- 2 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 issues/a-jobs-capture-holds-every-programs-lines-of-its-window.md diff --git a/issues/a-jobs-capture-holds-every-programs-lines-of-its-window.md b/issues/a-jobs-capture-holds-every-programs-lines-of-its-window.md new file mode 100644 index 00000000000..f75ea600749 --- /dev/null +++ b/issues/a-jobs-capture-holds-every-programs-lines-of-its-window.md @@ -0,0 +1,57 @@ +--- +status: open +kind: tooling +opened: 2026-10-09 +--- + +# A job's capture holds every program's lines of its window, so a test that compares one whole is red under load only + +`TestResult::stdout` is filed by `push_user_half` (`tests/common/qemu.rs`), +which takes every console line between a job's `===TEST_START===` and its +end marker that is not the kernel's. Whose line it is is not asked: a server +still finishing its boot when the job starts says its late lines into the +job's capture, and the head `logkeeper` gave each, which names its program, +is cut off by `user_text` before the test sees it. A test that compares a +capture whole is green alone, green in one suite and red in the next, by how +late the host's load makes a server. + +Evidence: the suite at `6b1a9322f` exited 1 with 37 of 38, `uptime`'s +one-minute mean 29.6 as it began and 47.9 as it ended. +`nvme_disk_keeps_log_and_home` compared `cat` of a kept file with `cat` of +its source, byte for byte, as the first two jobs after a reboot's ready +marker, and said they differed. The kept file's read was its ten lines. The +source's was six of `acpiserver`'s boot lines and then the same ten, value +for value; as the round-2 review of pull request #797 read them off the +run's log, the six run from `table 1 of 1 (DSDT) loaded` to +`\_S5 handed to the kernel`: + +``` +acpiserver: table 1 of 1 (DSDT) loaded +… four more of acpiserver's … +acpiserver: \_S5 handed to the kernel… +NAME="…" +… nine more lines of the file, each `UPPER_CASE="…"` … +``` + +The four between and the head each line carried on the console were not +read again for this file. The same test passed alone at that head (exit 0) +and in the suite at `a4033a5d2` (exit 0, 38 of 38, load 56.8 to 45.1). It is +not reproduced on demand: no late line was staged. + +`9a0660e79` holds both reads to the lines of the file's own form: the +`assignments` closure in `nvme_disk_keeps_log_and_home` (`tests/toyos.rs`) +keeps a line whose key before `=` is upper case and `_` only, which leans on +no program saying such a line and no longer sees a line of another form +added to the kept file. That test was the tree's only whole-capture +comparison when this was filed: `run_test`'s other callers look for one +line, a phrase, or an exit code. + +**Exit condition.** `TestResult` hands a job its own lines, told from every +other program's by the head each line arrives with, and the `assignments` +filter is deleted from `nvme_disk_keeps_log_and_home`, which then compares +the two reads whole. The clause on `tests/CLAUDE.md`'s two-pieces bullet +that states this fact goes in the same change. + +## Owner + +`tests/common/qemu.rs`; unheld. diff --git a/tests/CLAUDE.md b/tests/CLAUDE.md index 9361f41e995..eec45466aae 100644 --- a/tests/CLAUDE.md +++ b/tests/CLAUDE.md @@ -9,7 +9,7 @@ The mechanics live where the work is: profiles and shapes in `tests/common/`, re - **A machine-wide death during boot is reproduced by boots, not by suites** — parallel `bootable.img` guests, each waited on its completion marker and never on a fixed timer; the baseline is measured in the same session as the arm; a death counts whether or not a marker printed, so run guests with `-action reboot=shutdown -action shutdown=pause` and read a silent one's registers over QMP; a defect whose rate is set by interrupts per unit of guest work is measured on the *slowest* instrument — TCG can be the stronger oracle. - **`/system/bin/supervisor` speaks in every program's name before that program runs** — a predicate keyed on a `: ` prefix is satisfied by the wrong speaker; wait for the whole line, in the constant the assertion also reads. - **A guest binary cannot ask what a handle it does not hold does** — the probe ends its caller with exit 139, so it runs in a child, one fault per child. -- **A boot's capture has two pieces** — `boot_log()` ends at the ready marker, `run_test`'s capture begins at `===TEST_START===`. +- **A boot's capture has two pieces** — `boot_log()` ends at the ready marker, `run_test`'s capture begins at `===TEST_START===`, and a job's capture is every program's lines of its window, a server's late boot lines among them. - **A guest of an architecture the host is not runs under TCG** (`Arch::accel`) — TCG prices an uncontended atomic read-modify-write unlike hardware, and an x86-64 guest there boots `qemu64`, which has no PCID: every `INVPCID` path is dead, so a change gated on a CPUID feature is unverified by a green TCG suite. - **A liveness ceiling scales by two host facts** — boot-derived host speed *and* the guest's own `vcpus/cores` oversubscription. Widen a *liveness* guard for this, never a correctness bound. - **A wedge verdict needs both the budget spent and the guest gone quiet** — a healthy idle guest can be silent for minutes, and a guest still talking past its budget is slow, not stuck; only a far backstop stands behind a guest that keeps talking.