From b5ee6b4d0896ec89dad4f7c8e8109fc4dc556f66 Mon Sep 17 00:00:00 2001 From: Pascal Scholz Date: Mon, 3 Aug 2026 14:11:14 +0200 Subject: [PATCH 1/9] X: use fw_cfg --- cloud-hypervisor/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cloud-hypervisor/Cargo.toml b/cloud-hypervisor/Cargo.toml index be61df65f6..fc53690086 100644 --- a/cloud-hypervisor/Cargo.toml +++ b/cloud-hypervisor/Cargo.toml @@ -47,7 +47,7 @@ wait-timeout = { workspace = true } # feature list below [features] dbus_api = ["vmm/dbus_api", "zbus"] -default = ["io_uring", "kvm"] +default = ["io_uring", "kvm", "fw_cfg"] dhat-heap = ["dhat", "vmm/dhat-heap"] # For heap profiling fw_cfg = ["vmm/fw_cfg"] guest_debug = ["vmm/guest_debug"] From 51746e14f07fa5a72409136cc4ed9f944c81ac40 Mon Sep 17 00:00:00 2001 From: Pascal Scholz Date: Fri, 7 Aug 2026 17:54:34 +0200 Subject: [PATCH 2/9] devices: Disable `fw_cfg` DMA transfers The DMA interface in `fw_cfg` in the CHV implementation is broken and needs an overhaul. We deactivate it to force a guest to use the traditional interface instead and ignore the DMA interface.[0] We make DMA transfers no-ops for now and adjust the test to verify this instead until we do a rework of the DMA path. [0] https://www.qemu.org/docs/master/specs/fw_cfg.html#guest-side-dma-interface On-behalf-of: SAP pascal.scholz@sap.com Signed-off-by: Pascal Scholz --- devices/src/legacy/fw_cfg.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/devices/src/legacy/fw_cfg.rs b/devices/src/legacy/fw_cfg.rs index c5200e5f50..f77a44b421 100644 --- a/devices/src/legacy/fw_cfg.rs +++ b/devices/src/legacy/fw_cfg.rs @@ -96,9 +96,10 @@ pub const FW_CFG_DMA_SIGNATURE: [u8; 8] = *b"QEMU CFG"; pub const FW_CFG_ACPI_ID: &str = "QEMU0002"; // Reserved (must be enabled) const FW_CFG_F_RESERVED: u8 = 1 << 0; -// DMA Toggle Bit (enabled by default) const FW_CFG_F_DMA: u8 = 1 << 1; -pub const FW_CFG_FEATURE: [u8; 4] = [FW_CFG_F_RESERVED | FW_CFG_F_DMA, 0, 0, 0]; +// We disable the broken DMA interface while the rework is in progress. +// See https://github.com/cobaltcore-dev/cobaltcore/issues/647. +pub const FW_CFG_FEATURE: [u8; 4] = [FW_CFG_F_RESERVED, 0, 0, 0]; const COMMAND_ALLOCATE: u32 = 0x1; const COMMAND_ADD_POINTER: u32 = 0x2; @@ -594,6 +595,11 @@ impl FwCfg { } fn do_dma(&mut self) { + // If the DMA bit is not set, then DMA is a no-op like Write from the traditional interface. + if (FW_CFG_FEATURE[0] & FW_CFG_F_DMA) == 0 { + return; + } + let dma_address = self.dma_address; let mut access = FwCfgDmaAccess::new_zeroed(); let dma_access = match self @@ -1006,6 +1012,9 @@ mod unit_tests { fw_cfg.write(0, DMA_OFFSET, &dma_lo); fw_cfg.write(0, DMA_OFFSET + 4, &dma_hi); let _ = mem.read(&mut data, GuestAddress(code_address)); - assert_eq!(data, code); + + // Assert that the DMA path is currently deactivated. + assert_eq!(data, [0u8; 12]); + assert_eq!(fw_cfg.data_offset, 0); } } From 6317169a1e9aa3e87a75ce7e74c588e7890800e1 Mon Sep 17 00:00:00 2001 From: Pascal Scholz Date: Mon, 3 Aug 2026 15:01:23 +0200 Subject: [PATCH 3/9] devices: Use correct signature for selector 0x0000 in `fw_cfg` The selector 0x0000 has to return the bytes "QEMU" in the traditional interface.[0] Each additional read beyond those four bytes should return 0x0. The previous implementation exposed the complete DMA signature if a guest reads more than four bytes from the selector 0x0000 instead. We fix this by treating each signature separately. [0] https://www.qemu.org/docs/master/specs/fw_cfg.html#signature-key-0x0000-fw-cfg-signature On-behalf-of: SAP pascal.scholz@sap.com Signed-off-by: Pascal Scholz --- devices/src/legacy/fw_cfg.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/devices/src/legacy/fw_cfg.rs b/devices/src/legacy/fw_cfg.rs index f77a44b421..75115cf328 100644 --- a/devices/src/legacy/fw_cfg.rs +++ b/devices/src/legacy/fw_cfg.rs @@ -3,7 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 // -/// Cloud Hypervisor implementation of Qemu's fw_cfg spec +/// Cloud Hypervisor implementation of QEMU's fw_cfg spec /// https://www.qemu.org/docs/master/specs/fw_cfg.html /// Linux kernel fw_cfg driver header /// https://github.com/torvalds/linux/blob/master/include/uapi/linux/qemu_fw_cfg.h @@ -91,7 +91,8 @@ const FW_CFG_FILE_DIR: u16 = 0x19; const FW_CFG_KNOWN_ITEMS: usize = 0x20; pub const FW_CFG_FILE_FIRST: u16 = 0x20; -pub const FW_CFG_DMA_SIGNATURE: [u8; 8] = *b"QEMU CFG"; +pub const FW_CFG_DMA_SIGNATURE_CONTENT: [u8; 8] = *b"QEMU CFG"; +pub const FW_CFG_SIGNATURE_CONTENT: [u8; 4] = *b"QEMU"; // https://github.com/torvalds/linux/blob/master/include/uapi/linux/qemu_fw_cfg.h pub const FW_CFG_ACPI_ID: &str = "QEMU0002"; // Reserved (must be enabled) @@ -420,7 +421,7 @@ impl FwCfg { pub fn new(memory: GuestMemoryAtomic>) -> FwCfg { const DEFAULT_ITEM: FwCfgContent = FwCfgContent::Slice(&[]); let mut known_items = [DEFAULT_ITEM; FW_CFG_KNOWN_ITEMS]; - known_items[FW_CFG_SIGNATURE as usize] = FwCfgContent::Slice(&FW_CFG_DMA_SIGNATURE); + known_items[FW_CFG_SIGNATURE as usize] = FwCfgContent::Slice(&FW_CFG_SIGNATURE_CONTENT); known_items[FW_CFG_ID as usize] = FwCfgContent::Slice(&FW_CFG_FEATURE); let file_buf = Vec::from(FwCfgFilesHeader { count_be: 0 }.as_mut_bytes()); known_items[FW_CFG_FILE_DIR as usize] = FwCfgContent::Bytes(file_buf); @@ -866,7 +867,7 @@ mod unit_tests { let mut data = vec![0u8]; - let mut sig_iter = FW_CFG_DMA_SIGNATURE.into_iter(); + let mut sig_iter = FW_CFG_SIGNATURE_CONTENT.into_iter(); fw_cfg.write(0, SELECTOR_OFFSET, &[FW_CFG_SIGNATURE as u8, 0]); loop { if let Some(char) = sig_iter.next() { @@ -877,6 +878,7 @@ mod unit_tests { } } } + #[test] fn test_kernel_cmdline() { let gm = GuestMemoryAtomic::new( From 8e79e165cb20b0dcbfa6914186f0f06607bdd278 Mon Sep 17 00:00:00 2001 From: Pascal Scholz Date: Fri, 7 Aug 2026 14:10:23 +0200 Subject: [PATCH 4/9] devices: Restrict x86 fw_cfg PIO reads to one byte In the traditional/PIO interface the DATA register has a width of one byte on x86.[0] We therefore do not allow reads with larger widths. Currently, all tests assume a read with a width of one byte too. We reject `fw_cfg` in aarch64 builds for now as these changes introduce an incompatibility that adds to the incomplete implementation of it. aarch64 support is a task to be solved in follow-up work, as this also includes making corrections to the MMIO transport implementation and FDT corrections. [0] https://www.qemu.org/docs/master/specs/fw_cfg.html#data-register On-behalf-of: SAP pascal.scholz@sap.com Signed-off-by: Pascal Scholz --- devices/src/legacy/fw_cfg.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/devices/src/legacy/fw_cfg.rs b/devices/src/legacy/fw_cfg.rs index 75115cf328..4095ef2486 100644 --- a/devices/src/legacy/fw_cfg.rs +++ b/devices/src/legacy/fw_cfg.rs @@ -179,6 +179,12 @@ pub struct FwCfgItem { pub content: FwCfgContent, } +// ARM MMIO transport needs a rework. +// Find more details here: https://github.com/cobaltcore-dev/cobaltcore/issues/650 +#[cfg(all(feature = "fw_cfg", target_arch = "aarch64"))] +compile_error!( + "fw_cfg is not supported on aarch64: the MMIO transport is incomplete and defective." +); /// https://www.qemu.org/docs/master/specs/fw_cfg.html #[derive(Debug)] pub struct FwCfg { @@ -778,7 +784,7 @@ impl BusDevice for FwCfg { (PORT_FW_CFG_SELECTOR, _) => { error!("fw_cfg: selector register is write-only."); } - (PORT_FW_CFG_DATA, _) => _ = self.read_data(data, size as u32), + (PORT_FW_CFG_DATA, 1) => _ = self.read_data(data, size as u32), (PORT_FW_CFG_DMA_HI, 4) => { let addr = self.dma_address; let addr_hi = (addr >> 32) as u32; From 19720198fa08bdce4176d50544581f05a03eb1c7 Mon Sep 17 00:00:00 2001 From: Pascal Scholz Date: Fri, 7 Aug 2026 17:55:20 +0200 Subject: [PATCH 5/9] devices: Return zero for invalid-width `fw_cfg` PIO reads QEMU checks if the read has an allowed width.[0] If it detects an invalid read, then QEMU will treat the register reads as a read from unassigned I/O.[1] As a result, it will return 0x0 for the whole buffer. QEMU maps two separate memory regions to fw_cfg. One starting at FW_CFG_IO_BASE which is 0x510 and one starting at 0x514.[2] The first is two bytes, the size of the latter is eight bytes. This leaves a hole at addresses 0x512 and 0x513 in the x86 I/O port mapping. We mimic this behavior for CHV's `fw_cfg` design for improved compatibility and respect the hole accordingly. [0] https://github.com/qemu/qemu/blob/6e9a825c1d4e7b62d072e99a89ecd1a74c7f0d55/hw/nvram/fw_cfg.c#L533 [1] https://github.com/qemu/qemu/blob/6e9a825c1d4e7b62d072e99a89ecd1a74c7f0d55/system/memory.c#L1480 [2] https://github.com/qemu/qemu/blob/6e9a825c1d4e7b62d072e99a89ecd1a74c7f0d55/hw/i386/fw_cfg.c#L130 On-behalf-of: SAP pascal.scholz@sap.com Signed-off-by: Pascal Scholz --- devices/src/legacy/fw_cfg.rs | 62 ++++++++++++++++++++++++++++++++++-- 1 file changed, 60 insertions(+), 2 deletions(-) diff --git a/devices/src/legacy/fw_cfg.rs b/devices/src/legacy/fw_cfg.rs index 4095ef2486..6a04f3b453 100644 --- a/devices/src/legacy/fw_cfg.rs +++ b/devices/src/legacy/fw_cfg.rs @@ -780,6 +780,8 @@ impl BusDevice for FwCfg { fn read(&mut self, _base: u64, offset: u64, data: &mut [u8]) { let port = offset + PORT_FW_CFG_BASE; let size = data.len(); + let mut qemu_mapped_offsets = (PORT_FW_CFG_SELECTOR..PORT_FW_CFG_DATA + 1) + .chain(PORT_FW_CFG_DMA_HI..PORT_FW_CFG_DMA_LO + 4); match (port, size) { (PORT_FW_CFG_SELECTOR, _) => { error!("fw_cfg: selector register is write-only."); @@ -795,9 +797,21 @@ impl BusDevice for FwCfg { let addr_lo = (addr & 0xffff_ffff) as u32; data.copy_from_slice(&addr_lo.to_be_bytes()); } - _ => { + (port, _) if qemu_mapped_offsets.any(|mapped_port| mapped_port == port) => { + // We read from a port that should actually be mapped to fw_cfg. Note that QEMU + // doesn't map the entire range but leaves a hole at 0x512 and 0x513. We mimic this + // by doing a no-op below for this range. debug!( - "fw_cfg: read from unknown port {port:#x}: {size:#x} bytes and offset {offset:#x}." + "fw_cfg: Unsupported {:#x}-byte read from port {port:#x}.", + data.len() + ); + data.fill(0x0); + } + (port, _) => { + // We read from a port that shouldn't be mapped to fw_cfg and do nothing but warn. + debug!( + "fw_cfg: Unsupported {:#x}-byte read from unsupported port {port:#x}. This is a wrong mapping!", + data.len() ); } } @@ -1025,4 +1039,48 @@ mod unit_tests { assert_eq!(data, [0u8; 12]); assert_eq!(fw_cfg.data_offset, 0); } + + #[test] + fn test_register_invalid_reads_zero_buffer() { + // Reads with unsupported size zero the whole buffer in QEMU. We mimic this behavior. + let mut fw_cfg = FwCfg::new(GuestMemoryAtomic::new(GuestMemoryMmap::new())); + fw_cfg.write(0, SELECTOR_OFFSET, &[FW_CFG_SIGNATURE as u8, 0]); + // Two-byte reads are forbidden. + let mut buff = [0xEF; 2]; + fw_cfg.read(0, DATA_OFFSET, &mut buff); + assert_eq!(fw_cfg.data_offset, 0); + assert_eq!(buff, [0x0; 2]); + // Four-byte reads are forbidden. + let mut buff = [0xEF; 4]; + fw_cfg.read(0, DATA_OFFSET, &mut buff); + assert_eq!(buff, [0x0; 4]); + assert_eq!(fw_cfg.data_offset, 0); + // One-byte reads return actual data. + let mut buff = [0xEF; 1]; + fw_cfg.read(0, DATA_OFFSET, &mut buff); + assert_eq!(fw_cfg.data_offset, 1); + assert_eq!(buff, [b'Q']); + } + + #[test] + fn test_register_invalid_ports_leaves_buffer_untouched() { + // We should not answer reads from unknown ports. + let mut fw_cfg = FwCfg::new(GuestMemoryAtomic::new(GuestMemoryMmap::new())); + fw_cfg.write(0, SELECTOR_OFFSET, &[FW_CFG_SIGNATURE as u8, 0]); + // Single-byte reads from forbidden ports should be a no-op. Test the address succeeding the + // mapped range of 0xC addresses. + let mut buff = [0xCD; 1]; + fw_cfg.read(0, PORT_FW_CFG_DMA_LO - PORT_FW_CFG_BASE + 4, &mut buff); + assert_eq!(fw_cfg.data_offset, 0); + assert_eq!(buff, [0xCD; 1]); + // Test that reads to addresses in the hole of the mapping are no-ops too. + let mut buff = [0xCD; 1]; + fw_cfg.read(0, PORT_FW_CFG_DATA - PORT_FW_CFG_BASE + 1, &mut buff); + assert_eq!(fw_cfg.data_offset, 0); + assert_eq!(buff, [0xCD; 1]); + let mut buff = [0xCD; 1]; + fw_cfg.read(0, PORT_FW_CFG_DATA - PORT_FW_CFG_BASE + 2, &mut buff); + assert_eq!(fw_cfg.data_offset, 0); + assert_eq!(buff, [0xCD; 1]); + } } From 788ba8cf378e8cc00d3715474d013a58206079ab Mon Sep 17 00:00:00 2001 From: Pascal Scholz Date: Fri, 7 Aug 2026 17:22:17 +0200 Subject: [PATCH 6/9] devices: Use offset semantics when matching bus addresses in fw_cfg Using the result of an addition comes with the risk of it overflowing. In terms of the address matching in `read` and `write` it makes no difference if we use the provided offset directly instead of adding the base to it. On-behalf-of: SAP pascal.scholz@sap.com Signed-off-by: Pascal Scholz --- devices/src/legacy/fw_cfg.rs | 131 ++++++++++++++++++----------------- 1 file changed, 68 insertions(+), 63 deletions(-) diff --git a/devices/src/legacy/fw_cfg.rs b/devices/src/legacy/fw_cfg.rs index 6a04f3b453..3dcd68131a 100644 --- a/devices/src/legacy/fw_cfg.rs +++ b/devices/src/legacy/fw_cfg.rs @@ -53,25 +53,25 @@ const E820_RAM: u32 = 1; const E820_RESERVED: u32 = 2; #[cfg(target_arch = "x86_64")] -const PORT_FW_CFG_SELECTOR: u64 = 0x510; +const PORT_FW_CFG_SELECTOR_OFFSET: u64 = 0x0; #[cfg(target_arch = "x86_64")] -const PORT_FW_CFG_DATA: u64 = 0x511; +const PORT_FW_CFG_DATA_OFFSET: u64 = 0x1; #[cfg(target_arch = "x86_64")] -const PORT_FW_CFG_DMA_HI: u64 = 0x514; +const PORT_FW_CFG_DMA_HI_OFFSET: u64 = 0x4; #[cfg(target_arch = "x86_64")] -const PORT_FW_CFG_DMA_LO: u64 = 0x518; +const PORT_FW_CFG_DMA_LO_OFFSET: u64 = 0x8; #[cfg(target_arch = "x86_64")] pub const PORT_FW_CFG_BASE: u64 = 0x510; #[cfg(target_arch = "x86_64")] pub const PORT_FW_CFG_WIDTH: u64 = 0xc; #[cfg(target_arch = "aarch64")] -const PORT_FW_CFG_SELECTOR: u64 = 0x9030008; +const PORT_FW_CFG_SELECTOR_OFFSET: u64 = 0x8; #[cfg(target_arch = "aarch64")] -const PORT_FW_CFG_DATA: u64 = 0x9030000; +const PORT_FW_CFG_DATA_OFFSET: u64 = 0x0; #[cfg(target_arch = "aarch64")] -const PORT_FW_CFG_DMA_HI: u64 = 0x9030010; +const PORT_FW_CFG_DMA_HI_OFFSET: u64 = 0x10; #[cfg(target_arch = "aarch64")] -const PORT_FW_CFG_DMA_LO: u64 = 0x9030014; +const PORT_FW_CFG_DMA_LO_OFFSET: u64 = 0x14; #[cfg(target_arch = "aarch64")] pub const PORT_FW_CFG_BASE: u64 = 0x9030000; #[cfg(target_arch = "aarch64")] @@ -778,50 +778,50 @@ impl FwCfg { impl BusDevice for FwCfg { fn read(&mut self, _base: u64, offset: u64, data: &mut [u8]) { - let port = offset + PORT_FW_CFG_BASE; let size = data.len(); - let mut qemu_mapped_offsets = (PORT_FW_CFG_SELECTOR..PORT_FW_CFG_DATA + 1) - .chain(PORT_FW_CFG_DMA_HI..PORT_FW_CFG_DMA_LO + 4); - match (port, size) { - (PORT_FW_CFG_SELECTOR, _) => { + let mut qemu_mapped_offsets = (PORT_FW_CFG_SELECTOR_OFFSET..PORT_FW_CFG_DATA_OFFSET + 1) + .chain(PORT_FW_CFG_DMA_HI_OFFSET..PORT_FW_CFG_DMA_LO_OFFSET + 4); + match (offset, size) { + (PORT_FW_CFG_SELECTOR_OFFSET, _) => { error!("fw_cfg: selector register is write-only."); } - (PORT_FW_CFG_DATA, 1) => _ = self.read_data(data, size as u32), - (PORT_FW_CFG_DMA_HI, 4) => { + (PORT_FW_CFG_DATA_OFFSET, 1) => _ = self.read_data(data, size as u32), + (PORT_FW_CFG_DMA_HI_OFFSET, 4) => { let addr = self.dma_address; let addr_hi = (addr >> 32) as u32; data.copy_from_slice(&addr_hi.to_be_bytes()); } - (PORT_FW_CFG_DMA_LO, 4) => { + (PORT_FW_CFG_DMA_LO_OFFSET, 4) => { let addr = self.dma_address; let addr_lo = (addr & 0xffff_ffff) as u32; data.copy_from_slice(&addr_lo.to_be_bytes()); } - (port, _) if qemu_mapped_offsets.any(|mapped_port| mapped_port == port) => { + (offset, _) if qemu_mapped_offsets.any(|mapped_offset| mapped_offset == offset) => { // We read from a port that should actually be mapped to fw_cfg. Note that QEMU // doesn't map the entire range but leaves a hole at 0x512 and 0x513. We mimic this // by doing a no-op below for this range. debug!( - "fw_cfg: Unsupported {:#x}-byte read from port {port:#x}.", - data.len() + "fw_cfg: Unsupported {:#x}-byte read from address: base={:#x} + offset={:#x}.", + data.len(), + PORT_FW_CFG_BASE, + offset ); + data.fill(0x0); } - (port, _) => { + (offset, _) => { // We read from a port that shouldn't be mapped to fw_cfg and do nothing but warn. debug!( - "fw_cfg: Unsupported {:#x}-byte read from unsupported port {port:#x}. This is a wrong mapping!", - data.len() + "fw_cfg: read to unmapped address: base={PORT_FW_CFG_BASE:#x} + offset={offset:#x}. Read length: {size}. This is a wrong mapping and a bug!" ); } } } fn write(&mut self, _base: u64, offset: u64, data: &[u8]) -> Option> { - let port = offset + PORT_FW_CFG_BASE; let size = data.size(); - match (port, size) { - (PORT_FW_CFG_SELECTOR, 2) => { + match (offset, size) { + (PORT_FW_CFG_SELECTOR_OFFSET, 2) => { let mut buf = [0u8; 2]; buf[..size].copy_from_slice(&data[..size]); #[cfg(target_arch = "x86_64")] @@ -831,15 +831,15 @@ impl BusDevice for FwCfg { self.selector = val; self.data_offset = 0; } - (PORT_FW_CFG_DATA, 1) => error!("fw_cfg: data register is read-only."), - (PORT_FW_CFG_DMA_HI, 4) => { + (PORT_FW_CFG_DATA_OFFSET, 1) => error!("fw_cfg: data register is read-only."), + (PORT_FW_CFG_DMA_HI_OFFSET, 4) => { let mut buf = [0u8; 4]; buf[..size].copy_from_slice(&data[..size]); let val = u32::from_be_bytes(buf); self.dma_address &= 0xffff_ffff; self.dma_address |= (val as u64) << 32; } - (PORT_FW_CFG_DMA_LO, 4) => { + (PORT_FW_CFG_DMA_LO_OFFSET, 4) => { let mut buf = [0u8; 4]; buf[..size].copy_from_slice(&data[..size]); let val = u32::from_be_bytes(buf); @@ -847,9 +847,11 @@ impl BusDevice for FwCfg { self.dma_address |= val as u64; self.do_dma(); } - _ => debug!( - "fw_cfg: write to unknown port {port:#x}: {size:#x} bytes and offset {offset:#x} ." - ), + _ => { + debug!( + "fw_cfg: write to unmapped address: base={PORT_FW_CFG_BASE:#x} + offset={offset:#x}. Write length: {size}. This is a wrong mapping and a bug!" + ); + } } None } @@ -864,19 +866,6 @@ mod unit_tests { use super::*; - #[cfg(target_arch = "x86_64")] - const SELECTOR_OFFSET: u64 = 0; - #[cfg(target_arch = "aarch64")] - const SELECTOR_OFFSET: u64 = 8; - #[cfg(target_arch = "x86_64")] - const DATA_OFFSET: u64 = 1; - #[cfg(target_arch = "aarch64")] - const DATA_OFFSET: u64 = 0; - #[cfg(target_arch = "x86_64")] - const DMA_OFFSET: u64 = 4; - #[cfg(target_arch = "aarch64")] - const DMA_OFFSET: u64 = 16; - #[test] fn test_signature() { let gm = GuestMemoryAtomic::new( @@ -888,10 +877,10 @@ mod unit_tests { let mut data = vec![0u8]; let mut sig_iter = FW_CFG_SIGNATURE_CONTENT.into_iter(); - fw_cfg.write(0, SELECTOR_OFFSET, &[FW_CFG_SIGNATURE as u8, 0]); + fw_cfg.write(0, PORT_FW_CFG_SELECTOR_OFFSET, &[FW_CFG_SIGNATURE as u8, 0]); loop { if let Some(char) = sig_iter.next() { - fw_cfg.read(0, DATA_OFFSET, &mut data); + fw_cfg.read(0, PORT_FW_CFG_DATA_OFFSET, &mut data); assert_eq!(data[0], char); } else { return; @@ -914,10 +903,14 @@ mod unit_tests { let mut data = vec![0u8]; let mut cmdline_iter = cmdline.into_iter(); - fw_cfg.write(0, SELECTOR_OFFSET, &[FW_CFG_CMDLINE_DATA as u8, 0]); + fw_cfg.write( + 0, + PORT_FW_CFG_SELECTOR_OFFSET, + &[FW_CFG_CMDLINE_DATA as u8, 0], + ); loop { if let Some(char) = cmdline_iter.next() { - fw_cfg.read(0, DATA_OFFSET, &mut data); + fw_cfg.read(0, PORT_FW_CFG_DATA_OFFSET, &mut data); assert_eq!(data[0], char); } else { return; @@ -944,10 +937,14 @@ mod unit_tests { let mut data = vec![0u8]; let mut initram_iter = (*initram_content).into_iter(); - fw_cfg.write(0, SELECTOR_OFFSET, &[FW_CFG_INITRD_DATA as u8, 0]); + fw_cfg.write( + 0, + PORT_FW_CFG_SELECTOR_OFFSET, + &[FW_CFG_INITRD_DATA as u8, 0], + ); loop { if let Some(char) = initram_iter.next() { - fw_cfg.read(0, DATA_OFFSET, &mut data); + fw_cfg.read(0, PORT_FW_CFG_DATA_OFFSET, &mut data); assert_eq!(data[0], char); } else { return; @@ -974,9 +971,13 @@ mod unit_tests { let mut data = vec![0u8]; // Select the first file item (FW_CFG_FILE_FIRST = 0x20) - fw_cfg.write(0, SELECTOR_OFFSET, &[FW_CFG_FILE_FIRST as u8, 0]); + fw_cfg.write( + 0, + PORT_FW_CFG_SELECTOR_OFFSET, + &[FW_CFG_FILE_FIRST as u8, 0], + ); for &byte in expected.iter() { - fw_cfg.read(0, DATA_OFFSET, &mut data); + fw_cfg.read(0, PORT_FW_CFG_DATA_OFFSET, &mut data); assert_eq!(data[0], byte); } } @@ -1030,9 +1031,13 @@ mod unit_tests { let _ = mem.read(&mut data, GuestAddress(code_address)); assert_ne!(data, code); - fw_cfg.write(0, SELECTOR_OFFSET, &[FW_CFG_FILE_FIRST as u8, 0]); - fw_cfg.write(0, DMA_OFFSET, &dma_lo); - fw_cfg.write(0, DMA_OFFSET + 4, &dma_hi); + fw_cfg.write( + 0, + PORT_FW_CFG_SELECTOR_OFFSET, + &[FW_CFG_FILE_FIRST as u8, 0], + ); + fw_cfg.write(0, PORT_FW_CFG_DMA_LO_OFFSET, &dma_lo); + fw_cfg.write(0, PORT_FW_CFG_DMA_HI_OFFSET, &dma_hi); let _ = mem.read(&mut data, GuestAddress(code_address)); // Assert that the DMA path is currently deactivated. @@ -1044,20 +1049,20 @@ mod unit_tests { fn test_register_invalid_reads_zero_buffer() { // Reads with unsupported size zero the whole buffer in QEMU. We mimic this behavior. let mut fw_cfg = FwCfg::new(GuestMemoryAtomic::new(GuestMemoryMmap::new())); - fw_cfg.write(0, SELECTOR_OFFSET, &[FW_CFG_SIGNATURE as u8, 0]); + fw_cfg.write(0, PORT_FW_CFG_SELECTOR_OFFSET, &[FW_CFG_SIGNATURE as u8, 0]); // Two-byte reads are forbidden. let mut buff = [0xEF; 2]; - fw_cfg.read(0, DATA_OFFSET, &mut buff); + fw_cfg.read(0, PORT_FW_CFG_DATA_OFFSET, &mut buff); assert_eq!(fw_cfg.data_offset, 0); assert_eq!(buff, [0x0; 2]); // Four-byte reads are forbidden. let mut buff = [0xEF; 4]; - fw_cfg.read(0, DATA_OFFSET, &mut buff); + fw_cfg.read(0, PORT_FW_CFG_DATA_OFFSET, &mut buff); assert_eq!(buff, [0x0; 4]); assert_eq!(fw_cfg.data_offset, 0); // One-byte reads return actual data. let mut buff = [0xEF; 1]; - fw_cfg.read(0, DATA_OFFSET, &mut buff); + fw_cfg.read(0, PORT_FW_CFG_DATA_OFFSET, &mut buff); assert_eq!(fw_cfg.data_offset, 1); assert_eq!(buff, [b'Q']); } @@ -1066,20 +1071,20 @@ mod unit_tests { fn test_register_invalid_ports_leaves_buffer_untouched() { // We should not answer reads from unknown ports. let mut fw_cfg = FwCfg::new(GuestMemoryAtomic::new(GuestMemoryMmap::new())); - fw_cfg.write(0, SELECTOR_OFFSET, &[FW_CFG_SIGNATURE as u8, 0]); + fw_cfg.write(0, PORT_FW_CFG_SELECTOR_OFFSET, &[FW_CFG_SIGNATURE as u8, 0]); // Single-byte reads from forbidden ports should be a no-op. Test the address succeeding the // mapped range of 0xC addresses. let mut buff = [0xCD; 1]; - fw_cfg.read(0, PORT_FW_CFG_DMA_LO - PORT_FW_CFG_BASE + 4, &mut buff); + fw_cfg.read(0, PORT_FW_CFG_DMA_LO_OFFSET + 4, &mut buff); assert_eq!(fw_cfg.data_offset, 0); assert_eq!(buff, [0xCD; 1]); // Test that reads to addresses in the hole of the mapping are no-ops too. let mut buff = [0xCD; 1]; - fw_cfg.read(0, PORT_FW_CFG_DATA - PORT_FW_CFG_BASE + 1, &mut buff); + fw_cfg.read(0, PORT_FW_CFG_DATA_OFFSET + 1, &mut buff); assert_eq!(fw_cfg.data_offset, 0); assert_eq!(buff, [0xCD; 1]); let mut buff = [0xCD; 1]; - fw_cfg.read(0, PORT_FW_CFG_DATA - PORT_FW_CFG_BASE + 2, &mut buff); + fw_cfg.read(0, PORT_FW_CFG_DATA_OFFSET + 2, &mut buff); assert_eq!(fw_cfg.data_offset, 0); assert_eq!(buff, [0xCD; 1]); } From def5352e8ebcb67395e977982b3c354c88133563 Mon Sep 17 00:00:00 2001 From: Pascal Scholz Date: Fri, 7 Aug 2026 14:19:28 +0200 Subject: [PATCH 7/9] devices: Mimic QEMU `fw_cfg` selector-read quirk While the documentation states that the SELECTOR register is write-only, QEMU actually allows reading the SELECTOR register. This is because QEMU uses a contiguous mapping for the SELECTOR and DATA registers to allow the 16-bit width of the SELECTOR register.[0] As a consequence, a read from SELECTOR is delegated to the same callback as a read from DATA. It does not return the SELECTOR value. We mimic this for maximal compatibility. [0] https://github.com/qemu/qemu/blob/6e9a825c1d4e7b62d072e99a89ecd1a74c7f0d55/hw/nvram/fw_cfg.c#L539 On-behalf-of: SAP pascal.scholz@sap.com Signed-off-by: Pascal Scholz --- devices/src/legacy/fw_cfg.rs | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/devices/src/legacy/fw_cfg.rs b/devices/src/legacy/fw_cfg.rs index 3dcd68131a..87bf2f22b5 100644 --- a/devices/src/legacy/fw_cfg.rs +++ b/devices/src/legacy/fw_cfg.rs @@ -782,8 +782,10 @@ impl BusDevice for FwCfg { let mut qemu_mapped_offsets = (PORT_FW_CFG_SELECTOR_OFFSET..PORT_FW_CFG_DATA_OFFSET + 1) .chain(PORT_FW_CFG_DMA_HI_OFFSET..PORT_FW_CFG_DMA_LO_OFFSET + 4); match (offset, size) { - (PORT_FW_CFG_SELECTOR_OFFSET, _) => { - error!("fw_cfg: selector register is write-only."); + (PORT_FW_CFG_SELECTOR_OFFSET, 1) => { + // Selector register is actually defined write-only. QEMU’s combined PIO region + // treats a 1-byte read at this offset as a data read. Bypass to mimic QEMU quirk. + self.read_data(data, size as u32); } (PORT_FW_CFG_DATA_OFFSET, 1) => _ = self.read_data(data, size as u32), (PORT_FW_CFG_DMA_HI_OFFSET, 4) => { @@ -1088,4 +1090,23 @@ mod unit_tests { assert_eq!(fw_cfg.data_offset, 0); assert_eq!(buff, [0xCD; 1]); } + + #[test] + fn test_register_qemu_selector_read_quirk() { + // While defined as write-only, QEMU uses a port-mapping that leaves the select register + // readable. For full compatibility we also allow reading from the selector register as a + // quirk. + let mut fw_cfg = FwCfg::new(GuestMemoryAtomic::new(GuestMemoryMmap::new())); + fw_cfg.write(0, PORT_FW_CFG_SELECTOR_OFFSET, &[FW_CFG_SIGNATURE as u8, 0]); + // One-byte read returns actual data. + let mut buff = [0xEF; 1]; + fw_cfg.read(0, PORT_FW_CFG_SELECTOR_OFFSET, &mut buff); + assert_eq!(fw_cfg.data_offset, 1); + assert_eq!(buff, [b'Q']); + // Forbidden access zeros buffer similar to data register access. Offset isn't moved. + let mut buff = [0xEF; 2]; + fw_cfg.read(0, PORT_FW_CFG_SELECTOR_OFFSET, &mut buff); + assert_eq!(fw_cfg.data_offset, 1); + assert_eq!(buff, [0x0; 2]); + } } From b3540af509bc3dde385a5065cf954c2cffe32d32 Mon Sep 17 00:00:00 2001 From: Pascal Scholz Date: Fri, 7 Aug 2026 18:32:37 +0200 Subject: [PATCH 8/9] devices: Rework `fw_cfg` traditional/PIO interface reads Currently, the read implementation isn't complete and doesn't handle some error cases gracefully. We rework it with the aim of maximal compatibility to QEMU and add tests for it. Problems of the old implementation include: * Reads beyond EOF should yield 0x0.[0] These currently panic. * Register reads with invalid SELECTOR should also yield 0x0.[1] * If provided with a buffer larger than an item, then the remaining buffer bytes should be set to zero.[1] [0] https://www.qemu.org/docs/master/specs/fw_cfg.html#data-register [1] https://github.com/qemu/qemu/blob/6e9a825c1d4e7b62d072e99a89ecd1a74c7f0d55/hw/nvram/fw_cfg.c#L382 On-behalf-of: SAP pascal.scholz@sap.com Signed-off-by: Pascal Scholz --- devices/src/legacy/fw_cfg.rs | 209 ++++++++++++++++++++++++++++------- 1 file changed, 169 insertions(+), 40 deletions(-) diff --git a/devices/src/legacy/fw_cfg.rs b/devices/src/legacy/fw_cfg.rs index 87bf2f22b5..03b52dca28 100644 --- a/devices/src/legacy/fw_cfg.rs +++ b/devices/src/legacy/fw_cfg.rs @@ -36,6 +36,7 @@ use linux_loader::bootparam::boot_params; #[cfg(target_arch = "aarch64")] use linux_loader::loader::pe::arm64_image_header as boot_params; use log::{debug, error}; +use thiserror::Error; use vm_device::BusDevice; use vm_memory::bitmap::AtomicBitmap; use vm_memory::{ @@ -423,6 +424,34 @@ fn create_acpi_loader(acpi_table: AcpiTable) -> [FwCfgItem; 3] { [table_loader, acpi_rsdp, apci_tables] } +#[derive(Error, Debug)] +pub enum FwCfgContentAccessError { + /// Failed to access the data source that is backing the FwCfg item. + #[error("Reading the source failed")] + ReadError, + /// The GPA turned out to be unmapped. + #[error("Accessing guest memory for DMA failed")] + GuestMemAccessError, + #[error("DMA target guest physical address is illegal")] + IllegalGpa, + /// Guest memory was unmapped when accessing the it at an address succeeding the + /// provided start GPA for DMA. + #[error("Cannot access the whole guest memory region for DMA")] + GuestMemOutOfBoundsAccess(u32), + /// FwCfg doesn't hold an item that can be referenced by the given selector. + #[error("There is no item accessible through the selector {0}")] + IllegalSelector(u16), + /// FwCfg's internal cursor points to the EOF, indicating that all bytes were read before. + #[error("The cursor already points to the item's end")] + CursorBehindContent, + /// The item accessed is too large and it's size cannot be represented by a 32-bit unsigned + /// integer. + #[error("The accessed item is too large")] + TooLarge, +} + +type FwCfgContentAccessResult = std::result::Result; + impl FwCfg { pub fn new(memory: GuestMemoryAtomic>) -> FwCfg { const DEFAULT_ITEM: FwCfgContent = FwCfgContent::Slice(&[]); @@ -563,6 +592,21 @@ impl FwCfg { Ok(()) } + /// Retrieves the [`FwCfgContent`] corresponding to the selector currently set in the internal + /// selector buffer. + fn get_selected_content(&self) -> FwCfgContentAccessResult<&FwCfgContent> { + if let Some(known_item) = self.known_items.get(usize::from(self.selector)) { + Ok(known_item) + } else if let Some(item) = self + .items + .get(usize::from(self.selector - FW_CFG_FILE_FIRST)) + { + Ok(&item.content) + } else { + Err(FwCfgContentAccessError::IllegalSelector(self.selector)) + } + } + fn dma_read_content( &self, content: &FwCfgContent, @@ -733,61 +777,69 @@ impl FwCfg { Ok(()) } - fn read_content(content: &FwCfgContent, offset: u32, data: &mut [u8], size: u32) -> Option { - let start = offset as usize; - let end = start + size as usize; - match content { - FwCfgContent::Bytes(b) => { - if b.len() >= size as usize { - data.copy_from_slice(&b[start..end]); - } - } - FwCfgContent::Slice(s) => { - if s.len() >= size as usize { - data.copy_from_slice(&s[start..end]); - } - } - FwCfgContent::File(o, f) => { - f.read_exact_at(data, o + offset as u64).ok()?; - } - FwCfgContent::U32(n) => { - let bytes = n.to_le_bytes(); - data.copy_from_slice(&bytes[start..end]); - } + /// Reads the data [`FwCfgContent`] of item currently selected through the internal selector + /// buffer to an externally provided buffer. + /// + /// On success, returns the number of bytes written to the buffer. This can be fewer bytes than + /// the buffer length, if the items content shorter than the buffer. If the buffer is shorter + /// than the item's content, then more than one reads is necessary to retrieve all data. + /// + /// Either accumulate the number of bytes returned through all calls to this function or use the + /// internal buffer for offset and the items size to determine if the all bytes were read. + /// + /// Errors if access to a file backed item fails ([`FwCfgContentAccessError::ReadError`]) or if + /// the size of the item exceeds u32::MAX ([`FwCfgContentAccessError::TooLarge]). + fn read_content(&mut self, data: &mut [u8]) -> FwCfgContentAccessResult { + let content_size = self + .get_selected_content()? + .size() + .map_err(|_| FwCfgContentAccessError::TooLarge)?; + + let remaining_content_bytes = content_size.saturating_sub(self.data_offset); + let content_bytes_to_copy = u32::min(remaining_content_bytes, data.len() as u32); + let planned_end = self.data_offset + content_bytes_to_copy; + let read_size = self + .get_selected_content()? + .access(self.data_offset) + .read(data[..content_bytes_to_copy as usize].as_mut_bytes()) + .map_err(|_| FwCfgContentAccessError::ReadError)?; + + // Only relevant for file backed items. These can change between + // access so the data used to calculate can be stale. We cannot fix this. + if read_size != content_bytes_to_copy as usize { + return Err(FwCfgContentAccessError::ReadError); } - Some(size as u8) + + self.data_offset = planned_end; + + Ok(content_bytes_to_copy) } - fn read_data(&mut self, data: &mut [u8], size: u32) -> u8 { - let ret = if let Some(content) = self.known_items.get(self.selector as usize) { - Self::read_content(content, self.data_offset, data, size) - } else if let Some(item) = self.items.get((self.selector - FW_CFG_FILE_FIRST) as usize) { - Self::read_content(&item.content, self.data_offset, data, size) - } else { - error!("fw_cfg: selector {:#x} does not exist.", self.selector); - None - }; - if let Some(val) = ret { - self.data_offset += size; - val + /// Read data from this [`FwCfg`]'s item selected through the internal selector buffer and write + /// it's data to the provided buffer. + /// + /// If less bytes were read from the item than the buffer can hold, remaining bytes of the + /// buffer will be filled with zeros (0x0). + fn read_data(&mut self, data: &mut [u8]) { + if let Ok(read_len) = self.read_content(data) { + data[read_len as usize..].fill(0x0); } else { - 0 + data.fill(0x0); } } } impl BusDevice for FwCfg { fn read(&mut self, _base: u64, offset: u64, data: &mut [u8]) { - let size = data.len(); let mut qemu_mapped_offsets = (PORT_FW_CFG_SELECTOR_OFFSET..PORT_FW_CFG_DATA_OFFSET + 1) .chain(PORT_FW_CFG_DMA_HI_OFFSET..PORT_FW_CFG_DMA_LO_OFFSET + 4); - match (offset, size) { + match (offset, data.len()) { (PORT_FW_CFG_SELECTOR_OFFSET, 1) => { // Selector register is actually defined write-only. QEMU’s combined PIO region // treats a 1-byte read at this offset as a data read. Bypass to mimic QEMU quirk. - self.read_data(data, size as u32); + self.read_data(data); } - (PORT_FW_CFG_DATA_OFFSET, 1) => _ = self.read_data(data, size as u32), + (PORT_FW_CFG_DATA_OFFSET, 1) => self.read_data(data), (PORT_FW_CFG_DMA_HI_OFFSET, 4) => { let addr = self.dma_address; let addr_hi = (addr >> 32) as u32; @@ -814,7 +866,8 @@ impl BusDevice for FwCfg { (offset, _) => { // We read from a port that shouldn't be mapped to fw_cfg and do nothing but warn. debug!( - "fw_cfg: read to unmapped address: base={PORT_FW_CFG_BASE:#x} + offset={offset:#x}. Read length: {size}. This is a wrong mapping and a bug!" + "fw_cfg: read to unmapped address: base={PORT_FW_CFG_BASE:#x} + offset={offset:#x}. Read length: {}. This is a wrong mapping and a bug!", + data.len() ); } } @@ -1109,4 +1162,80 @@ mod unit_tests { assert_eq!(fw_cfg.data_offset, 1); assert_eq!(buff, [0x0; 2]); } + + #[test] + fn test_register_reads_past_eof_return_zero() { + let mut fw_cfg = FwCfg::new(GuestMemoryAtomic::new(GuestMemoryMmap::new())); + fw_cfg.write(0, PORT_FW_CFG_SELECTOR_OFFSET, &[FW_CFG_SIGNATURE as u8, 0]); + let mut buff = [0xEF; 8]; + let max_offset = FW_CFG_SIGNATURE_CONTENT.len() as u32; + for (offset, byte) in buff.iter_mut().enumerate() { + fw_cfg.read(0, PORT_FW_CFG_DATA_OFFSET, byte.as_mut_bytes()); + let expected_offset = if (offset as u32 + 1) < max_offset { + offset as u32 + 1 + } else { + max_offset + }; + assert_eq!(fw_cfg.data_offset, expected_offset); + } + assert_eq!(buff[..4], FW_CFG_SIGNATURE_CONTENT); + assert_eq!(buff[4..], [0; 4]); + } + + #[test] + fn test_register_reads_with_invalid_selector() { + const SELECTOR_INITIALIZED_WITH_DEFAULT: u16 = 0x08; + let mut fw_cfg = FwCfg::new(GuestMemoryAtomic::new(GuestMemoryMmap::new())); + fw_cfg.known_items[SELECTOR_INITIALIZED_WITH_DEFAULT as usize] = FwCfgContent::Slice(&[]); + fw_cfg.write(0, PORT_FW_CFG_SELECTOR_OFFSET, &[0xFF, 0]); + let mut buff = [0xEF_u8; 8]; + for byte in buff.iter_mut() { + fw_cfg.read(0, PORT_FW_CFG_DATA_OFFSET, byte.as_mut_bytes()); + assert_eq!(fw_cfg.data_offset, 0); + } + assert_eq!(buff, [0; 8]); + + fw_cfg.write( + 0, + PORT_FW_CFG_SELECTOR_OFFSET, + &SELECTOR_INITIALIZED_WITH_DEFAULT.to_le_bytes(), + ); + let mut buff = [0xEF_u8; 8]; + for byte in buff.iter_mut() { + fw_cfg.read(0, PORT_FW_CFG_DATA_OFFSET, byte.as_mut_bytes()); + assert_eq!(fw_cfg.data_offset, 0); + } + assert_eq!(buff, [0; 8]); + } + + #[test] + fn test_register_writing_select_resets_internal_cursor() { + let mut fw_cfg = FwCfg::new(GuestMemoryAtomic::new(GuestMemoryMmap::new())); + let payload_bytes = [0x11, 0x22, 0x33, 0x44, 0x55, 0x66]; + let content = FwCfgContent::Bytes(payload_bytes.to_vec()); + let cfg_item = FwCfgItem { + name: "payload".to_string(), + content, + }; + fw_cfg.add_item(cfg_item).unwrap(); + + // read the same bytes twice, demonstrating that we can reset the cursor by selecting a new item. + for _ in 0..2 { + fw_cfg.write( + 0, + PORT_FW_CFG_SELECTOR_OFFSET, + &FW_CFG_FILE_FIRST.to_le_bytes(), + ); + assert_eq!(fw_cfg.data_offset, 0); + let mut buffer = [0xEF_u8; 6]; + const MAX_INDEX: usize = 4; + for (index, byte) in buffer.iter_mut().enumerate().take(MAX_INDEX) { + fw_cfg.read(0, PORT_FW_CFG_DATA_OFFSET, byte.as_mut_bytes()); + assert_eq!(fw_cfg.data_offset as usize, index + 1); + } + assert_eq!(buffer[..MAX_INDEX], payload_bytes[..MAX_INDEX]); + assert_eq!(buffer[MAX_INDEX..], [0xEF; 2]); + assert_eq!(fw_cfg.data_offset, MAX_INDEX as u32); + } + } } From 09e43f298f05109616e3472d9c259752f62a6c8b Mon Sep 17 00:00:00 2001 From: Pascal Scholz Date: Fri, 7 Aug 2026 18:35:27 +0200 Subject: [PATCH 9/9] devices: Allow arbitrary length reads from fw_cfg DATA This is a fix to allow string reads that map to `rep ins`. KVM returns an I/O exit with the respective port, the read size of the instruction and the count of such reads. `kvm-ioctls` creates a buffer from this with the size of `count * size` bytes.[0] This makes it impossible to decide if the buffer was a single four-byte-width read of the kind `inl` or `rep ins` with RCX set to 4, for example. While the first would be invalid according to QEMU semantics, the second is a valid repeated access with one byte width. We therefore accept reads of any size until we can solve this issue. [0] https://github.com/rust-vmm/kvm/blob/b4c9ed8df95a9e10a68f50f5ef5e7d04108759ba/kvm-ioctls/src/ioctls/vcpu.rs#L1549 On-behalf-of: SAP pascal.scholz@sap.com Signed-off-by: Pascal Scholz --- devices/src/legacy/fw_cfg.rs | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/devices/src/legacy/fw_cfg.rs b/devices/src/legacy/fw_cfg.rs index 03b52dca28..d324a91902 100644 --- a/devices/src/legacy/fw_cfg.rs +++ b/devices/src/legacy/fw_cfg.rs @@ -839,7 +839,11 @@ impl BusDevice for FwCfg { // treats a 1-byte read at this offset as a data read. Bypass to mimic QEMU quirk. self.read_data(data); } - (PORT_FW_CFG_DATA_OFFSET, 1) => self.read_data(data), + // TODO: For now we need to allow arbitrary length reads from DATA because we cannot + // distinguish between on one multi byte long read and multiple single-byte reads. + // There is an open issue in kvm-ioctls: https://github.com/rust-vmm/kvm/issues/371 + // Once this is solved, we should only support one-byte-length reads. + (PORT_FW_CFG_DATA_OFFSET, _) => self.read_data(data), (PORT_FW_CFG_DMA_HI_OFFSET, 4) => { let addr = self.dma_address; let addr_hi = (addr >> 32) as u32; @@ -1101,25 +1105,27 @@ mod unit_tests { } #[test] - fn test_register_invalid_reads_zero_buffer() { - // Reads with unsupported size zero the whole buffer in QEMU. We mimic this behavior. + fn test_register_allow_arbitrary_length_reads_from_data() { let mut fw_cfg = FwCfg::new(GuestMemoryAtomic::new(GuestMemoryMmap::new())); fw_cfg.write(0, PORT_FW_CFG_SELECTOR_OFFSET, &[FW_CFG_SIGNATURE as u8, 0]); - // Two-byte reads are forbidden. + + // Two-byte reads are served. let mut buff = [0xEF; 2]; fw_cfg.read(0, PORT_FW_CFG_DATA_OFFSET, &mut buff); - assert_eq!(fw_cfg.data_offset, 0); - assert_eq!(buff, [0x0; 2]); - // Four-byte reads are forbidden. + assert_eq!(buff, *b"QE"); + assert_eq!(fw_cfg.data_offset, 2); + // Four-byte reads are served. let mut buff = [0xEF; 4]; + fw_cfg.write(0, PORT_FW_CFG_SELECTOR_OFFSET, &[FW_CFG_SIGNATURE as u8, 0]); fw_cfg.read(0, PORT_FW_CFG_DATA_OFFSET, &mut buff); - assert_eq!(buff, [0x0; 4]); - assert_eq!(fw_cfg.data_offset, 0); - // One-byte reads return actual data. - let mut buff = [0xEF; 1]; + assert_eq!(buff, *b"QEMU"); + assert_eq!(fw_cfg.data_offset, 4); + // Eight-byte reads are served. + let mut buff = [0xEF; 8]; + fw_cfg.write(0, PORT_FW_CFG_SELECTOR_OFFSET, &[FW_CFG_SIGNATURE as u8, 0]); fw_cfg.read(0, PORT_FW_CFG_DATA_OFFSET, &mut buff); - assert_eq!(fw_cfg.data_offset, 1); - assert_eq!(buff, [b'Q']); + assert_eq!(buff, *b"QEMU\0\0\0\0"); + assert_eq!(fw_cfg.data_offset, 4); } #[test]