From 1d3e883e37902136f2cad46665866a229025a39e Mon Sep 17 00:00:00 2001 From: Leander Kohler Date: Fri, 29 May 2026 09:55:43 +0200 Subject: [PATCH 01/15] Revert "arch: smbios: add tests for table serialization" This reverts commit 9e3afb7d69a0117c575a0b86c74c88952434588e. On-behalf-of: SAP leander.kohler@sap.com Signed-off-by: Leander Kohler --- arch/src/x86_64/smbios.rs | 225 +------------------------------------- 1 file changed, 3 insertions(+), 222 deletions(-) diff --git a/arch/src/x86_64/smbios.rs b/arch/src/x86_64/smbios.rs index a59c39fef3..eacfd2a67f 100644 --- a/arch/src/x86_64/smbios.rs +++ b/arch/src/x86_64/smbios.rs @@ -443,55 +443,8 @@ pub fn setup_smbios(mem: &GuestMemoryMmap, smbios: Option<&SmbiosConfig>) -> Res mod unit_tests { use super::*; - /// Collects all strings after a SMBIOS structure, stopping at the double-NUL terminator and returns next addr. - fn read_string_set(mem: &GuestMemoryMmap, addr: GuestAddress) -> (Vec, GuestAddress) { - let mut cur = addr; - let read_byte = |addr: GuestAddress| -> u8 { mem.read_obj(addr).unwrap() }; - - // SMBIOS string-set: NUL-terminated strings, terminated by an extra NUL. - // Empty string-set is exactly "\0\0". - if read_byte(cur) == 0 { - let next = cur.checked_add(1).unwrap(); - assert_eq!(read_byte(next), 0); - return (Vec::new(), next.checked_add(1).unwrap()); - } - - let mut strings = Vec::new(); - loop { - let mut bytes = Vec::new(); - loop { - let b = read_byte(cur); - cur = cur.checked_add(1).unwrap(); - if b == 0 { - break; - } - bytes.push(b); - } - strings.push(String::from_utf8(bytes).unwrap()); - - // If the next byte is NUL, that's the extra terminator. - if read_byte(cur) == 0 { - cur = cur.checked_add(1).unwrap(); - break; - } - } - - (strings, cur) - } - - #[test] - fn entrypoint_checksum() { - let mem = GuestMemoryMmap::from_ranges(&[(GuestAddress(SMBIOS_START), 4096)]).unwrap(); - - setup_smbios(&mem, None).unwrap(); - - let smbios_ep: Smbios30Entrypoint = mem.read_obj(GuestAddress(SMBIOS_START)).unwrap(); - - assert_eq!(compute_checksum(&smbios_ep), 0); - } - #[test] - fn entrypoint_struct_size() { + fn struct_size() { assert_eq!( mem::size_of::(), 0x18usize, @@ -510,185 +463,13 @@ mod unit_tests { } #[test] - fn smbios_chassis_empty_string_set_has_double_null() { - let mem = GuestMemoryMmap::from_ranges(&[(GuestAddress(SMBIOS_START), 4096)]).unwrap(); - let smbios = SmbiosConfig { - chassis: Some(SmbiosChassisConfig::default()), - ..Default::default() - }; - - setup_smbios(&mem, Some(&smbios)).unwrap(); - - let smbios_ep: Smbios30Entrypoint = mem.read_obj(GuestAddress(SMBIOS_START)).unwrap(); - let mut cur = GuestAddress(smbios_ep.physptr); - - let bios: SmbiosBiosInfo = mem.read_obj(cur).unwrap(); - cur = cur.checked_add(bios.length as u64).unwrap(); - let (_, next) = read_string_set(&mem, cur); - cur = next; - - let sys: SmbiosSysInfo = mem.read_obj(cur).unwrap(); - cur = cur.checked_add(sys.length as u64).unwrap(); - let (_, next) = read_string_set(&mem, cur); - cur = next; - - let chassis: SmbiosChassis = mem.read_obj(cur).unwrap(); - cur = cur.checked_add(chassis.length as u64).unwrap(); - // SMBIOS DSP0134 §6.1.3: empty string-set ends with double NUL. - let b0: u8 = mem.read_obj(cur).unwrap(); - let b1: u8 = mem.read_obj(cur.checked_add(1).unwrap()).unwrap(); - assert_eq!(b0, 0); - assert_eq!(b1, 0); - cur = cur.checked_add(2).unwrap(); - - let end: SmbiosEndOfTable = mem.read_obj(cur).unwrap(); - assert_eq!(end.r#type, END_OF_TABLE); - } - - #[test] - fn smbios_chassis_oem_strings_layout() { - let mem = GuestMemoryMmap::from_ranges(&[(GuestAddress(SMBIOS_START), 4096)]).unwrap(); - - let smbios = SmbiosConfig { - chassis: Some(SmbiosChassisConfig { - asset_tag: Some("rack1".to_string()), - ..Default::default() - }), - oem_strings: vec!["o1".to_string(), "o2".to_string()], - ..Default::default() - }; - - setup_smbios(&mem, Some(&smbios)).unwrap(); - - let smbios_ep: Smbios30Entrypoint = mem.read_obj(GuestAddress(SMBIOS_START)).unwrap(); - let mut cur = GuestAddress(smbios_ep.physptr); - - let bios: SmbiosBiosInfo = mem.read_obj(cur).unwrap(); - cur = cur.checked_add(bios.length as u64).unwrap(); - let (_, next) = read_string_set(&mem, cur); - cur = next; - - let sys: SmbiosSysInfo = mem.read_obj(cur).unwrap(); - cur = cur.checked_add(sys.length as u64).unwrap(); - let (_, next) = read_string_set(&mem, cur); - cur = next; - - let chassis: SmbiosChassis = mem.read_obj(cur).unwrap(); - assert_eq!(chassis.r#type, SYSTEM_ENCLOSURE); - assert_eq!(chassis.asset_tag, 1); - cur = cur.checked_add(chassis.length as u64).unwrap(); - let (chassis_strings, next) = read_string_set(&mem, cur); - assert_eq!(chassis_strings, vec!["rack1"]); - cur = next; - - let oem: SmbiosOemStrings = mem.read_obj(cur).unwrap(); - assert_eq!(oem.r#type, OEM_STRINGS); - assert_eq!(oem.count, 2); - cur = cur.checked_add(oem.length as u64).unwrap(); - let (oem_strings, next) = read_string_set(&mem, cur); - assert_eq!(oem_strings, vec!["o1", "o2"]); - cur = next; - - let end: SmbiosEndOfTable = mem.read_obj(cur).unwrap(); - assert_eq!(end.r#type, END_OF_TABLE); - } - - #[test] - fn smbios_strings_terminators_default() { + fn entrypoint_checksum() { let mem = GuestMemoryMmap::from_ranges(&[(GuestAddress(SMBIOS_START), 4096)]).unwrap(); setup_smbios(&mem, None).unwrap(); let smbios_ep: Smbios30Entrypoint = mem.read_obj(GuestAddress(SMBIOS_START)).unwrap(); - let mut cur = GuestAddress(smbios_ep.physptr); - - let bios: SmbiosBiosInfo = mem.read_obj(cur).unwrap(); - assert_eq!(bios.r#type, BIOS_INFORMATION); - cur = cur.checked_add(bios.length as u64).unwrap(); - let (bios_strings, next) = read_string_set(&mem, cur); - assert_eq!(bios_strings, vec!["cloud-hypervisor", "0"]); - cur = next; - - let sys: SmbiosSysInfo = mem.read_obj(cur).unwrap(); - assert_eq!(sys.r#type, SYSTEM_INFORMATION); - assert_eq!(sys.manufacturer, 1); - assert_eq!(sys.product_name, 2); - assert_eq!(sys.version, 0); - assert_eq!(sys.serial_number, 0); - assert_eq!(sys.sku, 0); - assert_eq!(sys.family, 0); - cur = cur.checked_add(sys.length as u64).unwrap(); - let (sys_strings, next) = read_string_set(&mem, cur); - assert_eq!( - sys_strings, - vec![DEFAULT_SYSTEM_MANUFACTURER, DEFAULT_SYSTEM_PRODUCT_NAME] - ); - cur = next; - let end: SmbiosEndOfTable = mem.read_obj(cur).unwrap(); - assert_eq!(end.r#type, END_OF_TABLE); - } - - #[test] - fn smbios_strings_too_many() { - let mut next = 1u8; - for _ in 0..255 { - alloc_index(&mut next, true).unwrap(); - } - let err = alloc_index(&mut next, true).unwrap_err(); - assert!(matches!(err, Error::TooManyStrings)); - } - - #[test] - fn smbios_uuid_invalid_rejected() { - let mem = GuestMemoryMmap::from_ranges(&[(GuestAddress(SMBIOS_START), 4096)]).unwrap(); - let smbios = SmbiosConfig { - system: Some(SmbiosSystem { - uuid: Some("not-a-uuid".to_string()), - ..Default::default() - }), - ..Default::default() - }; - - let err = setup_smbios(&mem, Some(&smbios)).unwrap_err(); - assert!(matches!(err, Error::ParseUuid(_, _))); - } - - #[test] - fn smbios_uuid_written_le() { - let mem = GuestMemoryMmap::from_ranges(&[(GuestAddress(SMBIOS_START), 4096)]).unwrap(); - let uuid_str = "00112233-4455-6677-8899-aabbccddeeff"; - let smbios = SmbiosConfig { - system: Some(SmbiosSystem { - uuid: Some(uuid_str.to_string()), - ..Default::default() - }), - ..Default::default() - }; - - setup_smbios(&mem, Some(&smbios)).unwrap(); - - let smbios_ep: Smbios30Entrypoint = mem.read_obj(GuestAddress(SMBIOS_START)).unwrap(); - let mut cur = GuestAddress(smbios_ep.physptr); - - let bios: SmbiosBiosInfo = mem.read_obj(cur).unwrap(); - cur = cur.checked_add(bios.length as u64).unwrap(); - let (_, next) = read_string_set(&mem, cur); - cur = next; - - let sys: SmbiosSysInfo = mem.read_obj(cur).unwrap(); - assert_eq!(sys.uuid, Uuid::parse_str(uuid_str).unwrap().to_bytes_le()); - } - - #[test] - fn smbios_write_fails_with_too_small_memory() { - let mem = GuestMemoryMmap::from_ranges(&[( - GuestAddress(SMBIOS_START), - mem::size_of::(), - )]) - .unwrap(); - - let err = setup_smbios(&mem, None).unwrap_err(); - assert!(matches!(err, Error::WriteData)); + assert_eq!(compute_checksum(&smbios_ep), 0); } } From 8368feef32b5003b8cf5106f6163d5b4f7e91ba3 Mon Sep 17 00:00:00 2001 From: Leander Kohler Date: Fri, 29 May 2026 09:57:59 +0200 Subject: [PATCH 02/15] Revert "vmm: deprecate legacy SMBIOS keys in API and CLI" This reverts commit 29fab1950ed02265ddc5aa9aac1d11068206527c. On-behalf-of: SAP leander.kohler@sap.com Signed-off-by: Leander Kohler --- vmm/src/api/openapi/cloud-hypervisor.yaml | 2 -- vmm/src/config.rs | 6 ------ 2 files changed, 8 deletions(-) diff --git a/vmm/src/api/openapi/cloud-hypervisor.yaml b/vmm/src/api/openapi/cloud-hypervisor.yaml index fd5ccec531..5ef14737fb 100644 --- a/vmm/src/api/openapi/cloud-hypervisor.yaml +++ b/vmm/src/api/openapi/cloud-hypervisor.yaml @@ -798,12 +798,10 @@ components: type: string serial_number: type: string - deprecated: true system_uuid: type: string uuid: type: string - deprecated: true oem_strings: type: array items: diff --git a/vmm/src/config.rs b/vmm/src/config.rs index d7d1283067..576159fd93 100644 --- a/vmm/src/config.rs +++ b/vmm/src/config.rs @@ -1002,9 +1002,6 @@ impl PlatformConfig { let legacy_serial_number = parser .convert::("serial_number") .map_err(Error::ParsePlatform)?; - if legacy_serial_number.is_some() { - warn!("'serial_number' in --platform is deprecated; use 'system_serial_number'."); - } platform_config.system_serial_number = platform_config .system_serial_number .or(legacy_serial_number); @@ -1012,9 +1009,6 @@ impl PlatformConfig { let legacy_uuid = parser .convert::("uuid") .map_err(Error::ParsePlatform)?; - if legacy_uuid.is_some() { - warn!("'uuid' in --platform is deprecated; use 'system_uuid'."); - } platform_config.system_uuid = platform_config.system_uuid.or(legacy_uuid); #[cfg(feature = "tdx")] let tdx = parser From c4b0081a77f69bf76dcb36508f7663a1059faacd Mon Sep 17 00:00:00 2001 From: Leander Kohler Date: Fri, 29 May 2026 09:58:15 +0200 Subject: [PATCH 03/15] Revert "vmm: platform: add structured SMBIOS config" This reverts commit 287c2c0c63999b2990fb1f5fc48b73bae1a7bb38. On-behalf-of: SAP leander.kohler@sap.com Signed-off-by: Leander Kohler --- arch/src/x86_64/mod.rs | 2 +- arch/src/x86_64/smbios.rs | 156 ++---------------- .../tests/common/tests_wrappers.rs | 6 +- vmm/src/api/openapi/cloud-hypervisor.yaml | 16 -- vmm/src/config.rs | 130 ++------------- vmm/src/vm_config.rs | 55 +----- 6 files changed, 39 insertions(+), 326 deletions(-) diff --git a/arch/src/x86_64/mod.rs b/arch/src/x86_64/mod.rs index b8c10d9e0e..0671e17fab 100644 --- a/arch/src/x86_64/mod.rs +++ b/arch/src/x86_64/mod.rs @@ -36,7 +36,7 @@ use linux_loader::loader::elf::start_info::{ use log::{debug, error, info, trace}; pub use msr_filter::{MAX_BITMAP_SIZE, filter_denied_msrs}; use serde::{Deserialize, Serialize}; -pub use smbios::{SmbiosChassisConfig, SmbiosConfig, SmbiosSystem}; +pub use smbios::SmbiosConfig; use thiserror::Error; use vm_memory::{ Address, Bytes, GuestAddress, GuestAddressSpace, GuestMemory, GuestMemoryAtomic, diff --git a/arch/src/x86_64/smbios.rs b/arch/src/x86_64/smbios.rs index eacfd2a67f..eb99de94d9 100644 --- a/arch/src/x86_64/smbios.rs +++ b/arch/src/x86_64/smbios.rs @@ -35,9 +35,6 @@ pub enum Error { /// Failure to parse uuid, uuid format may be error #[error("Failure to parse uuid: {1}")] ParseUuid(#[source] uuid::Error, String), - /// SMBIOS string index overflow (u8 limit reached). - #[error("SMBIOS string index overflow (u8 limit reached)")] - TooManyStrings, } pub type Result = result::Result; @@ -47,7 +44,6 @@ const SM3_MAGIC_IDENT: &[u8; 5usize] = b"_SM3_"; const BIOS_INFORMATION: u8 = 0; const SYSTEM_INFORMATION: u8 = 1; const OEM_STRINGS: u8 = 11; -const SYSTEM_ENCLOSURE: u8 = 3; const END_OF_TABLE: u8 = 127; const PCI_SUPPORTED: u64 = 1 << 7; const IS_VIRTUAL_MACHINE: u8 = 1 << 4; @@ -56,29 +52,9 @@ pub const DEFAULT_SYSTEM_PRODUCT_NAME: &str = "cloud-hypervisor"; #[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct SmbiosConfig { - pub system: Option, - pub chassis: Option, - pub oem_strings: Vec, -} - -#[derive(Clone, Debug, Default, PartialEq, Eq)] -pub struct SmbiosSystem { - pub manufacturer: Option, - pub product_name: Option, - pub version: Option, pub serial_number: Option, pub uuid: Option, - pub sku_number: Option, - pub family: Option, -} - -#[derive(Clone, Debug, Default, PartialEq, Eq)] -pub struct SmbiosChassisConfig { - pub manufacturer: Option, - pub chassis_type: Option, - pub version: Option, - pub serial_number: Option, - pub asset_tag: Option, + pub oem_strings: Vec, } fn compute_checksum(v: &T) -> u8 { @@ -148,33 +124,6 @@ struct SmbiosOemStrings { count: u8, } -/// SMBIOS Chassis Table (Type 3) as defined in DMTF SMBIOS 3.9.0: -/// https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.9.0.pdf -/// Note: trailing fields are omitted, so this structure is not complete. -#[repr(C, packed)] -#[derive(Default, Copy, Clone)] -struct SmbiosChassis { - r#type: u8, - length: u8, - handle: u16, - manufacturer: u8, - chassis_type: u8, - version: u8, - serial_number: u8, - asset_tag: u8, - bootup_state: u8, - power_supply_state: u8, - thermal_state: u8, - security_status: u8, - oem_defined: u32, - height: u8, - number_of_power_cords: u8, - contained_element_count: u8, - contained_element_record_length: u8, - // followed by contained element records (optional, variable-length) - // followed by sku_number: u8, rack_type: u8, rack_height: u8 -} - #[repr(C, packed)] #[derive(Default, Copy, Clone)] struct SmbiosEndOfTable { @@ -192,8 +141,6 @@ unsafe impl ByteValued for SmbiosSysInfo {} // SAFETY: data structure only contain a series of integers unsafe impl ByteValued for SmbiosOemStrings {} // SAFETY: data structure only contain a series of integers -unsafe impl ByteValued for SmbiosChassis {} -// SAFETY: data structure only contain a series of integers unsafe impl ByteValued for SmbiosEndOfTable {} fn write_and_incr( @@ -247,115 +194,44 @@ fn write_string_terminator( } } -fn alloc_index(next: &mut u8, present: bool) -> Result { - if !present { - return Ok(0); - } - - let idx = *next; - if idx == 0 { - // wrapped around, next starts always initially at 1 - return Err(Error::TooManyStrings); - } - - *next = next.wrapping_add(1); - Ok(idx) -} - fn write_type1_system( mem: &GuestMemoryMmap, curptr: &mut GuestAddress, handle: &mut u16, - system: Option<&SmbiosSystem>, + serial_number: Option<&str>, + uuid: Option<&str>, ) -> Result<()> { *handle += 1; - let manufacturer = system - .and_then(|s| s.manufacturer.as_deref()) - .unwrap_or(DEFAULT_SYSTEM_MANUFACTURER); - let product = system - .and_then(|s| s.product_name.as_deref()) - .unwrap_or(DEFAULT_SYSTEM_PRODUCT_NAME); - let version = system.and_then(|s| s.version.as_deref()); - let serial = system.and_then(|s| s.serial_number.as_deref()); - let uuid = system.and_then(|s| s.uuid.as_deref()); - let sku = system.and_then(|s| s.sku_number.as_deref()); - let family = system.and_then(|s| s.family.as_deref()); - let uuid_number = uuid .map(Uuid::parse_str) .transpose() .map_err(|e| Error::ParseUuid(e, uuid.unwrap().to_string()))? .unwrap_or(Uuid::nil()); + let serial_idx = serial_number.map(|_| 3).unwrap_or_default(); - let mut next = 1u8; - let manufacturer_idx = alloc_index(&mut next, true)?; - let product_idx = alloc_index(&mut next, true)?; - let version_idx = alloc_index(&mut next, version.is_some())?; - let serial_idx = alloc_index(&mut next, serial.is_some())?; - let sku_idx = alloc_index(&mut next, sku.is_some())?; - let family_idx = alloc_index(&mut next, family.is_some())?; - - let sys = SmbiosSysInfo { + let smbios_sysinfo = SmbiosSysInfo { r#type: SYSTEM_INFORMATION, length: mem::size_of::() as u8, handle: *handle, - manufacturer: manufacturer_idx, - product_name: product_idx, - version: version_idx, + manufacturer: 1, // First string written in this section + product_name: 2, // Second string written in this section serial_number: serial_idx, uuid: uuid_number.to_bytes_le(), - sku: sku_idx, - family: family_idx, ..Default::default() }; - *curptr = write_and_incr(mem, sys, *curptr)?; - *curptr = write_string(mem, manufacturer, *curptr)?; - *curptr = write_string(mem, product, *curptr)?; - *curptr = write_opt_string(mem, version, *curptr)?; - *curptr = write_opt_string(mem, serial, *curptr)?; - *curptr = write_opt_string(mem, sku, *curptr)?; - *curptr = write_opt_string(mem, family, *curptr)?; + *curptr = write_and_incr(mem, smbios_sysinfo, *curptr)?; + *curptr = write_string(mem, DEFAULT_SYSTEM_MANUFACTURER, *curptr)?; + *curptr = write_string(mem, DEFAULT_SYSTEM_PRODUCT_NAME, *curptr)?; + *curptr = write_opt_string(mem, serial_number, *curptr)?; *curptr = write_and_incr(mem, 0u8, *curptr)?; Ok(()) } -fn write_type3_chassis( - mem: &GuestMemoryMmap, - curptr: &mut GuestAddress, - handle: &mut u16, - chassis: &SmbiosChassisConfig, -) -> Result<()> { - *handle += 1; - - let asset_tag = chassis.asset_tag.as_deref(); - let mut next = 1u8; - let asset_idx = alloc_index(&mut next, asset_tag.is_some())?; - - let ch = SmbiosChassis { - r#type: SYSTEM_ENCLOSURE, - length: mem::size_of::() as u8, - handle: *handle, - manufacturer: 0, - chassis_type: 0, - version: 0, - serial_number: 0, - asset_tag: asset_idx, - contained_element_count: 0, - contained_element_record_length: 0, - ..Default::default() - }; - - *curptr = write_and_incr(mem, ch, *curptr)?; - *curptr = write_opt_string(mem, asset_tag, *curptr)?; - *curptr = write_string_terminator(mem, *curptr, asset_tag.is_some())?; - Ok(()) -} - pub fn setup_smbios(mem: &GuestMemoryMmap, smbios: Option<&SmbiosConfig>) -> Result { - let system = smbios.and_then(|cfg| cfg.system.as_ref()); - let chassis = smbios.and_then(|cfg| cfg.chassis.as_ref()); + let serial_number = smbios.and_then(|cfg| cfg.serial_number.as_deref()); + let uuid = smbios.and_then(|cfg| cfg.uuid.as_deref()); let oem_strings: &[String] = smbios.map_or(&[] as &[String], |cfg| cfg.oem_strings.as_slice()); let physptr = GuestAddress(SMBIOS_START) .checked_add(mem::size_of::() as u64) @@ -381,11 +257,7 @@ pub fn setup_smbios(mem: &GuestMemoryMmap, smbios: Option<&SmbiosConfig>) -> Res curptr = write_and_incr(mem, 0u8, curptr)?; } - write_type1_system(mem, &mut curptr, &mut handle, system)?; - - if let Some(chassis) = chassis { - write_type3_chassis(mem, &mut curptr, &mut handle, chassis)?; - } + write_type1_system(mem, &mut curptr, &mut handle, serial_number, uuid)?; if !oem_strings.is_empty() { handle += 1; diff --git a/cloud-hypervisor/tests/common/tests_wrappers.rs b/cloud-hypervisor/tests/common/tests_wrappers.rs index 27e4c1f272..693702b552 100644 --- a/cloud-hypervisor/tests/common/tests_wrappers.rs +++ b/cloud-hypervisor/tests/common/tests_wrappers.rs @@ -2219,7 +2219,7 @@ pub(crate) fn _test_dmi_serial_number(guest: &Guest) { let mut child = GuestCommand::new(guest) .default_cpus() .default_memory() - .default_kernel_cmdline_with_platform(Some("system_serial_number=a=b;c=d")) + .default_kernel_cmdline_with_platform(Some("serial_number=a=b;c=d")) .default_disks() .default_net() .capture_output() @@ -2248,9 +2248,7 @@ pub(crate) fn _test_dmi_uuid(guest: &Guest) { let mut child = GuestCommand::new(guest) .default_cpus() .default_memory() - .default_kernel_cmdline_with_platform(Some( - "system_uuid=1e8aa28a-435d-4027-87f4-40dceff1fa0a", - )) + .default_kernel_cmdline_with_platform(Some("uuid=1e8aa28a-435d-4027-87f4-40dceff1fa0a")) .default_disks() .default_net() .capture_output() diff --git a/vmm/src/api/openapi/cloud-hypervisor.yaml b/vmm/src/api/openapi/cloud-hypervisor.yaml index 5ef14737fb..d3c06ebf39 100644 --- a/vmm/src/api/openapi/cloud-hypervisor.yaml +++ b/vmm/src/api/openapi/cloud-hypervisor.yaml @@ -794,30 +794,14 @@ components: iommu_address_width: type: integer format: uint8 - system_serial_number: - type: string serial_number: type: string - system_uuid: - type: string uuid: type: string oem_strings: type: array items: type: string - system_manufacturer: - type: string - system_product_name: - type: string - system_version: - type: string - system_family: - type: string - system_sku_number: - type: string - chassis_asset_tag: - type: string tdx: type: boolean default: false diff --git a/vmm/src/config.rs b/vmm/src/config.rs index 576159fd93..c33670953c 100644 --- a/vmm/src/config.rs +++ b/vmm/src/config.rs @@ -844,11 +844,9 @@ impl PlatformConfig { static SYNTAX: LazyLock = LazyLock::new(|| { let mut syntax = "Platform configuration parameters \ \"num_pci_segments=,iommu_segments=,\ - iommu_address_width=,iommufd=on|off,vfio_p2p_dma=on|off,system_manufacturer=,\ - system_product_name=,system_version=,\ - system_serial_number=,system_uuid=,\ - system_sku_number=,system_family=,\ - oem_strings=,chassis_asset_tag=" + iommu_address_width=,serial_number=,\ + uuid=,oem_strings=,iommufd=on|off,\ + vfio_p2p_dma=on|off" .to_string(); if cfg!(feature = "tdx") { @@ -868,61 +866,16 @@ impl PlatformConfig { } pub fn parse(platform: &str) -> Result { - struct StringField { - key: &'static str, - apply: fn(&mut PlatformConfig, String), - } - - const SMBIOS_STRING_FIELDS: &[StringField] = &[ - StringField { - key: "system_manufacturer", - apply: |config, value| config.system_manufacturer = Some(value), - }, - StringField { - key: "system_product_name", - apply: |config, value| config.system_product_name = Some(value), - }, - StringField { - key: "system_version", - apply: |config, value| config.system_version = Some(value), - }, - StringField { - key: "system_serial_number", - apply: |config, value| config.system_serial_number = Some(value), - }, - StringField { - key: "system_uuid", - apply: |config, value| config.system_uuid = Some(value), - }, - StringField { - key: "system_sku_number", - apply: |config, value| config.system_sku_number = Some(value), - }, - StringField { - key: "system_family", - apply: |config, value| config.system_family = Some(value), - }, - StringField { - key: "chassis_asset_tag", - apply: |config, value| config.chassis_asset_tag = Some(value), - }, - ]; - let mut parser = OptionParser::new(); parser .add("num_pci_segments") .add("iommu_segments") .add("iommu_address_width") - .add("oem_strings") .add("serial_number") .add("uuid") .add("oem_strings") .add("iommufd") - .add("vfio_p2p_dma") - .add("uuid"); - for field in SMBIOS_STRING_FIELDS { - parser.add(field.key); - } + .add("vfio_p2p_dma"); #[cfg(feature = "tdx")] parser.add("tdx"); #[cfg(feature = "sev_snp")] @@ -941,6 +894,10 @@ impl PlatformConfig { .convert("iommu_address_width") .map_err(Error::ParsePlatform)? .unwrap_or(MAX_IOMMU_ADDRESS_WIDTH_BITS); + let serial_number = parser + .convert("serial_number") + .map_err(Error::ParsePlatform)?; + let uuid = parser.convert("uuid").map_err(Error::ParsePlatform)?; let oem_strings = parser .convert::("oem_strings") .map_err(Error::ParsePlatform)? @@ -968,71 +925,20 @@ impl PlatformConfig { .map_err(Error::ParsePlatform)? .unwrap_or(Toggle(false)) .0; - - let mut platform_config = PlatformConfig { + Ok(PlatformConfig { num_pci_segments, iommu_segments, iommu_address_width_bits, - system_serial_number: None, - system_uuid: None, + serial_number, + uuid, oem_strings, - system_manufacturer: None, - system_product_name: None, - system_version: None, - system_family: None, - system_sku_number: None, - chassis_asset_tag: None, iommufd, + vfio_p2p_dma, #[cfg(feature = "tdx")] tdx, #[cfg(feature = "sev_snp")] sev_snp, - vfio_p2p_dma, - }; - - for field in SMBIOS_STRING_FIELDS { - if let Some(value) = parser - .convert::(field.key) - .map_err(Error::ParsePlatform)? - { - (field.apply)(&mut platform_config, value); - } - } - - let legacy_serial_number = parser - .convert::("serial_number") - .map_err(Error::ParsePlatform)?; - platform_config.system_serial_number = platform_config - .system_serial_number - .or(legacy_serial_number); - - let legacy_uuid = parser - .convert::("uuid") - .map_err(Error::ParsePlatform)?; - platform_config.system_uuid = platform_config.system_uuid.or(legacy_uuid); - #[cfg(feature = "tdx")] - let tdx = parser - .convert::("tdx") - .map_err(Error::ParsePlatform)? - .unwrap_or(Toggle(false)) - .0; - #[cfg(feature = "sev_snp")] - let sev_snp = parser - .convert::("sev_snp") - .map_err(Error::ParsePlatform)? - .unwrap_or(Toggle(false)) - .0; - - #[cfg(feature = "tdx")] - { - platform_config.tdx = tdx; - } - #[cfg(feature = "sev_snp")] - { - platform_config.sev_snp = sev_snp; - } - - Ok(platform_config) + }) } pub fn validate(&self) -> ValidationResult<()> { @@ -5141,17 +5047,11 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" num_pci_segments: MAX_NUM_PCI_SEGMENTS, iommu_segments: None, iommu_address_width_bits: MAX_IOMMU_ADDRESS_WIDTH_BITS, - system_serial_number: None, - system_uuid: None, + serial_number: None, + uuid: None, oem_strings: Vec::new(), iommufd: false, vfio_p2p_dma: default_platformconfig_vfio_p2p_dma(), - system_manufacturer: None, - system_product_name: None, - system_version: None, - system_family: None, - system_sku_number: None, - chassis_asset_tag: None, #[cfg(feature = "tdx")] tdx: false, #[cfg(feature = "sev_snp")] diff --git a/vmm/src/vm_config.rs b/vmm/src/vm_config.rs index 8ea8f0e8ff..e4f681bb0f 100644 --- a/vmm/src/vm_config.rs +++ b/vmm/src/vm_config.rs @@ -129,24 +129,12 @@ pub struct PlatformConfig { pub iommu_segments: Option>, #[serde(default = "default_platformconfig_iommu_address_width_bits")] pub iommu_address_width_bits: u8, - #[serde(default, alias = "serial_number")] - pub system_serial_number: Option, - #[serde(default, alias = "uuid")] - pub system_uuid: Option, #[serde(default)] - pub oem_strings: Vec, - #[serde(default)] - pub system_manufacturer: Option, - #[serde(default)] - pub system_product_name: Option, - #[serde(default)] - pub system_version: Option, + pub serial_number: Option, #[serde(default)] - pub system_family: Option, + pub uuid: Option, #[serde(default)] - pub system_sku_number: Option, - #[serde(default)] - pub chassis_asset_tag: Option, + pub oem_strings: Vec, #[cfg(feature = "tdx")] #[serde(default)] pub tdx: bool, @@ -162,43 +150,14 @@ pub struct PlatformConfig { #[cfg(target_arch = "x86_64")] impl PlatformConfig { pub fn smbios_config(&self) -> Option { - let has_system = [ - &self.system_serial_number, - &self.system_uuid, - &self.system_manufacturer, - &self.system_product_name, - &self.system_version, - &self.system_family, - &self.system_sku_number, - ] - .iter() - .any(|v| v.is_some()); - - let system = has_system.then_some(arch::x86_64::SmbiosSystem { - manufacturer: self.system_manufacturer.clone(), - product_name: self.system_product_name.clone(), - version: self.system_version.clone(), - serial_number: self.system_serial_number.clone(), - uuid: self.system_uuid.clone(), - sku_number: self.system_sku_number.clone(), - family: self.system_family.clone(), - }); - - let chassis = - self.chassis_asset_tag - .clone() - .map(|asset_tag| arch::x86_64::SmbiosChassisConfig { - asset_tag: Some(asset_tag), - ..Default::default() - }); - let smbios = arch::x86_64::SmbiosConfig { - system, - chassis, + serial_number: self.serial_number.clone(), + uuid: self.uuid.clone(), oem_strings: self.oem_strings.clone(), }; - if smbios.system.is_none() && smbios.chassis.is_none() && smbios.oem_strings.is_empty() { + if smbios.serial_number.is_none() && smbios.uuid.is_none() && smbios.oem_strings.is_empty() + { None } else { Some(smbios) From 1a0647eb2d521ebdf69827d0866c9ad4a1ff046f Mon Sep 17 00:00:00 2001 From: Leander Kohler Date: Fri, 29 May 2026 09:58:23 +0200 Subject: [PATCH 04/15] Revert "vmm: plumb legacy SMBIOS config" This reverts commit 7a1d3310fbf12e69b08c5810d68fb1beddd3cf6a. On-behalf-of: SAP leander.kohler@sap.com Signed-off-by: Leander Kohler --- arch/src/x86_64/mod.rs | 16 +++++++++++++--- arch/src/x86_64/smbios.rs | 21 ++++++++------------- vmm/src/vm.rs | 25 ++++++++++++++++++++++--- vmm/src/vm_config.rs | 18 ------------------ 4 files changed, 43 insertions(+), 37 deletions(-) diff --git a/arch/src/x86_64/mod.rs b/arch/src/x86_64/mod.rs index 0671e17fab..c9778fba39 100644 --- a/arch/src/x86_64/mod.rs +++ b/arch/src/x86_64/mod.rs @@ -36,7 +36,6 @@ use linux_loader::loader::elf::start_info::{ use log::{debug, error, info, trace}; pub use msr_filter::{MAX_BITMAP_SIZE, filter_denied_msrs}; use serde::{Deserialize, Serialize}; -pub use smbios::SmbiosConfig; use thiserror::Error; use vm_memory::{ Address, Bytes, GuestAddress, GuestAddressSpace, GuestMemory, GuestMemoryAtomic, @@ -1260,7 +1259,9 @@ pub fn configure_system( _num_cpus: u32, setup_header: Option, rsdp_addr: Option, - smbios: Option<&SmbiosConfig>, + serial_number: Option<&str>, + uuid: Option<&str>, + oem_strings: Vec, topology: Option<(u16, u16, u16, u16)>, ) -> super::Result<()> { // Write EBDA address to location where ACPICA expects to find it @@ -1268,7 +1269,8 @@ pub fn configure_system( .write_obj((layout::EBDA_START.0 >> 4) as u16, layout::EBDA_POINTER) .map_err(Error::EbdaSetup)?; - let size = smbios::setup_smbios(guest_mem, smbios).map_err(Error::SmbiosSetup)?; + let size = smbios::setup_smbios(guest_mem, serial_number, uuid, oem_strings) + .map_err(Error::SmbiosSetup)?; // Place the MP table after the SMIOS table aligned to 16 bytes let offset = GuestAddress(layout::SMBIOS_START).unchecked_add(size); @@ -1813,6 +1815,8 @@ mod unit_tests { Some(layout::RSDP_POINTER), None, None, + Vec::new(), + None, ); config_err.unwrap_err(); @@ -1835,6 +1839,8 @@ mod unit_tests { None, None, None, + Vec::new(), + None, ) .unwrap(); @@ -1862,6 +1868,8 @@ mod unit_tests { None, None, None, + Vec::new(), + None, ) .unwrap(); @@ -1875,6 +1883,8 @@ mod unit_tests { None, None, None, + Vec::new(), + None, ) .unwrap(); } diff --git a/arch/src/x86_64/smbios.rs b/arch/src/x86_64/smbios.rs index eb99de94d9..cb8b331bb8 100644 --- a/arch/src/x86_64/smbios.rs +++ b/arch/src/x86_64/smbios.rs @@ -50,13 +50,6 @@ const IS_VIRTUAL_MACHINE: u8 = 1 << 4; pub const DEFAULT_SYSTEM_MANUFACTURER: &str = "Cloud Hypervisor"; pub const DEFAULT_SYSTEM_PRODUCT_NAME: &str = "cloud-hypervisor"; -#[derive(Clone, Debug, Default, PartialEq, Eq)] -pub struct SmbiosConfig { - pub serial_number: Option, - pub uuid: Option, - pub oem_strings: Vec, -} - fn compute_checksum(v: &T) -> u8 { let v: *const T = v; // SAFETY: we are only reading the bytes within the size of the `T` reference `v`. @@ -229,10 +222,12 @@ fn write_type1_system( Ok(()) } -pub fn setup_smbios(mem: &GuestMemoryMmap, smbios: Option<&SmbiosConfig>) -> Result { - let serial_number = smbios.and_then(|cfg| cfg.serial_number.as_deref()); - let uuid = smbios.and_then(|cfg| cfg.uuid.as_deref()); - let oem_strings: &[String] = smbios.map_or(&[] as &[String], |cfg| cfg.oem_strings.as_slice()); +pub fn setup_smbios( + mem: &GuestMemoryMmap, + serial_number: Option<&str>, + uuid: Option<&str>, + oem_strings: Vec, +) -> Result { let physptr = GuestAddress(SMBIOS_START) .checked_add(mem::size_of::() as u64) .ok_or(Error::NotEnoughMemory)?; @@ -272,7 +267,7 @@ pub fn setup_smbios(mem: &GuestMemoryMmap, smbios: Option<&SmbiosConfig>) -> Res curptr = write_and_incr(mem, smbios_oemstrings, curptr)?; for s in oem_strings { - curptr = write_string(mem, s, curptr)?; + curptr = write_string(mem, &s, curptr)?; } curptr = write_string_terminator(mem, curptr, true)?; @@ -338,7 +333,7 @@ mod unit_tests { fn entrypoint_checksum() { let mem = GuestMemoryMmap::from_ranges(&[(GuestAddress(SMBIOS_START), 4096)]).unwrap(); - setup_smbios(&mem, None).unwrap(); + setup_smbios(&mem, None, None, Vec::new()).unwrap(); let smbios_ep: Smbios30Entrypoint = mem.read_obj(GuestAddress(SMBIOS_START)).unwrap(); diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs index d62173e5fa..8e852d65ac 100644 --- a/vmm/src/vm.rs +++ b/vmm/src/vm.rs @@ -1922,13 +1922,30 @@ impl Vm { let boot_vcpus = self.cpu_manager.lock().unwrap().boot_vcpus(); - let smbios = self + let serial_number = self .config .lock() .unwrap() .platform .as_ref() - .and_then(|p| p.smbios_config()); + .and_then(|p| p.serial_number.clone()); + + let uuid = self + .config + .lock() + .unwrap() + .platform + .as_ref() + .and_then(|p| p.uuid.clone()); + + let oem_strings = self + .config + .lock() + .unwrap() + .platform + .as_ref() + .map(|p| p.oem_strings.clone()) + .unwrap_or_default(); let topology = self.cpu_manager.lock().unwrap().get_vcpu_topology(); @@ -1940,7 +1957,9 @@ impl Vm { boot_vcpus, entry_addr.setup_header, rsdp_addr, - smbios.as_ref(), + serial_number.as_deref(), + uuid.as_deref(), + oem_strings, topology, ) .map_err(Error::ConfigureSystem)?; diff --git a/vmm/src/vm_config.rs b/vmm/src/vm_config.rs index e4f681bb0f..f135b1e839 100644 --- a/vmm/src/vm_config.rs +++ b/vmm/src/vm_config.rs @@ -147,24 +147,6 @@ pub struct PlatformConfig { pub vfio_p2p_dma: bool, } -#[cfg(target_arch = "x86_64")] -impl PlatformConfig { - pub fn smbios_config(&self) -> Option { - let smbios = arch::x86_64::SmbiosConfig { - serial_number: self.serial_number.clone(), - uuid: self.uuid.clone(), - oem_strings: self.oem_strings.clone(), - }; - - if smbios.serial_number.is_none() && smbios.uuid.is_none() && smbios.oem_strings.is_empty() - { - None - } else { - Some(smbios) - } - } -} - pub const DEFAULT_PCI_SEGMENT_APERTURE_WEIGHT: u32 = 1; fn default_pci_segment_aperture_weight() -> u32 { From df71d35f9c19c77b611946559bc8ef4852c43d4f Mon Sep 17 00:00:00 2001 From: Leander Kohler Date: Fri, 29 May 2026 09:58:39 +0200 Subject: [PATCH 05/15] Revert "arch: x86_64: refactor SMBIOS helpers" This reverts commit 59763b39a022d740f377d94485bd521f561c0f63. On-behalf-of: SAP leander.kohler@sap.com Signed-off-by: Leander Kohler --- arch/src/x86_64/mod.rs | 10 ++-- arch/src/x86_64/smbios.rs | 116 ++++++++++++++------------------------ vmm/src/config.rs | 5 +- vmm/src/vm.rs | 9 ++- vmm/src/vm_config.rs | 2 +- 5 files changed, 55 insertions(+), 87 deletions(-) diff --git a/arch/src/x86_64/mod.rs b/arch/src/x86_64/mod.rs index c9778fba39..3e3152e129 100644 --- a/arch/src/x86_64/mod.rs +++ b/arch/src/x86_64/mod.rs @@ -1261,7 +1261,7 @@ pub fn configure_system( rsdp_addr: Option, serial_number: Option<&str>, uuid: Option<&str>, - oem_strings: Vec, + oem_strings: Option<&[&str]>, topology: Option<(u16, u16, u16, u16)>, ) -> super::Result<()> { // Write EBDA address to location where ACPICA expects to find it @@ -1815,7 +1815,7 @@ mod unit_tests { Some(layout::RSDP_POINTER), None, None, - Vec::new(), + None, None, ); config_err.unwrap_err(); @@ -1839,7 +1839,7 @@ mod unit_tests { None, None, None, - Vec::new(), + None, None, ) .unwrap(); @@ -1868,7 +1868,7 @@ mod unit_tests { None, None, None, - Vec::new(), + None, None, ) .unwrap(); @@ -1883,7 +1883,7 @@ mod unit_tests { None, None, None, - Vec::new(), + None, None, ) .unwrap(); diff --git a/arch/src/x86_64/smbios.rs b/arch/src/x86_64/smbios.rs index cb8b331bb8..6f1139888b 100644 --- a/arch/src/x86_64/smbios.rs +++ b/arch/src/x86_64/smbios.rs @@ -47,8 +47,6 @@ const OEM_STRINGS: u8 = 11; const END_OF_TABLE: u8 = 127; const PCI_SUPPORTED: u64 = 1 << 7; const IS_VIRTUAL_MACHINE: u8 = 1 << 4; -pub const DEFAULT_SYSTEM_MANUFACTURER: &str = "Cloud Hypervisor"; -pub const DEFAULT_SYSTEM_PRODUCT_NAME: &str = "cloud-hypervisor"; fn compute_checksum(v: &T) -> u8 { let v: *const T = v; @@ -61,7 +59,8 @@ fn compute_checksum(v: &T) -> u8 { (!checksum).wrapping_add(1) } -#[repr(C, packed)] +#[repr(C)] +#[repr(packed)] #[derive(Default, Copy, Clone)] struct Smbios30Entrypoint { signature: [u8; 5usize], @@ -76,7 +75,8 @@ struct Smbios30Entrypoint { physptr: u64, } -#[repr(C, packed)] +#[repr(C)] +#[repr(packed)] #[derive(Default, Copy, Clone)] struct SmbiosBiosInfo { r#type: u8, @@ -92,7 +92,8 @@ struct SmbiosBiosInfo { characteristics_ext2: u8, } -#[repr(C, packed)] +#[repr(C)] +#[repr(packed)] #[derive(Default, Copy, Clone)] struct SmbiosSysInfo { r#type: u8, @@ -108,7 +109,8 @@ struct SmbiosSysInfo { family: u8, } -#[repr(C, packed)] +#[repr(C)] +#[repr(packed)] #[derive(Default, Copy, Clone)] struct SmbiosOemStrings { r#type: u8, @@ -117,7 +119,8 @@ struct SmbiosOemStrings { count: u8, } -#[repr(C, packed)] +#[repr(C)] +#[repr(packed)] #[derive(Default, Copy, Clone)] struct SmbiosEndOfTable { r#type: u8, @@ -160,73 +163,11 @@ fn write_string( Ok(curptr) } -fn write_opt_string( - mem: &GuestMemoryMmap, - s: Option<&str>, - cur: GuestAddress, -) -> Result { - if let Some(v) = s { - write_string(mem, v, cur) - } else { - Ok(cur) - } -} - -fn write_string_terminator( - mem: &GuestMemoryMmap, - cur: GuestAddress, - has_strings: bool, -) -> Result { - // SMBIOS DSP0134 §6.1.3: if all string-reference fields are 0, follow the - // formatted section with two null bytes (empty string-set). - if has_strings { - write_and_incr(mem, 0u8, cur) - } else { - let cur = write_and_incr(mem, 0u8, cur)?; - write_and_incr(mem, 0u8, cur) - } -} - -fn write_type1_system( - mem: &GuestMemoryMmap, - curptr: &mut GuestAddress, - handle: &mut u16, - serial_number: Option<&str>, - uuid: Option<&str>, -) -> Result<()> { - *handle += 1; - - let uuid_number = uuid - .map(Uuid::parse_str) - .transpose() - .map_err(|e| Error::ParseUuid(e, uuid.unwrap().to_string()))? - .unwrap_or(Uuid::nil()); - let serial_idx = serial_number.map(|_| 3).unwrap_or_default(); - - let smbios_sysinfo = SmbiosSysInfo { - r#type: SYSTEM_INFORMATION, - length: mem::size_of::() as u8, - handle: *handle, - manufacturer: 1, // First string written in this section - product_name: 2, // Second string written in this section - serial_number: serial_idx, - uuid: uuid_number.to_bytes_le(), - ..Default::default() - }; - - *curptr = write_and_incr(mem, smbios_sysinfo, *curptr)?; - *curptr = write_string(mem, DEFAULT_SYSTEM_MANUFACTURER, *curptr)?; - *curptr = write_string(mem, DEFAULT_SYSTEM_PRODUCT_NAME, *curptr)?; - *curptr = write_opt_string(mem, serial_number, *curptr)?; - *curptr = write_and_incr(mem, 0u8, *curptr)?; - Ok(()) -} - pub fn setup_smbios( mem: &GuestMemoryMmap, serial_number: Option<&str>, uuid: Option<&str>, - oem_strings: Vec, + oem_strings: Option<&[&str]>, ) -> Result { let physptr = GuestAddress(SMBIOS_START) .checked_add(mem::size_of::() as u64) @@ -252,9 +193,34 @@ pub fn setup_smbios( curptr = write_and_incr(mem, 0u8, curptr)?; } - write_type1_system(mem, &mut curptr, &mut handle, serial_number, uuid)?; + { + handle += 1; - if !oem_strings.is_empty() { + let uuid_number = uuid + .map(Uuid::parse_str) + .transpose() + .map_err(|e| Error::ParseUuid(e, uuid.unwrap().to_string()))? + .unwrap_or(Uuid::nil()); + let smbios_sysinfo = SmbiosSysInfo { + r#type: SYSTEM_INFORMATION, + length: mem::size_of::() as u8, + handle, + manufacturer: 1, // First string written in this section + product_name: 2, // Second string written in this section + serial_number: serial_number.map(|_| 3).unwrap_or_default(), // 3rd string + uuid: uuid_number.to_bytes_le(), // set uuid + ..Default::default() + }; + curptr = write_and_incr(mem, smbios_sysinfo, curptr)?; + curptr = write_string(mem, "Cloud Hypervisor", curptr)?; + curptr = write_string(mem, "cloud-hypervisor", curptr)?; + if let Some(serial_number) = serial_number { + curptr = write_string(mem, serial_number, curptr)?; + } + curptr = write_and_incr(mem, 0u8, curptr)?; + } + + if let Some(oem_strings) = oem_strings { handle += 1; let smbios_oemstrings = SmbiosOemStrings { @@ -267,10 +233,10 @@ pub fn setup_smbios( curptr = write_and_incr(mem, smbios_oemstrings, curptr)?; for s in oem_strings { - curptr = write_string(mem, &s, curptr)?; + curptr = write_string(mem, s, curptr)?; } - curptr = write_string_terminator(mem, curptr, true)?; + curptr = write_and_incr(mem, 0u8, curptr)?; } { @@ -333,7 +299,7 @@ mod unit_tests { fn entrypoint_checksum() { let mem = GuestMemoryMmap::from_ranges(&[(GuestAddress(SMBIOS_START), 4096)]).unwrap(); - setup_smbios(&mem, None, None, Vec::new()).unwrap(); + setup_smbios(&mem, None, None, None).unwrap(); let smbios_ep: Smbios30Entrypoint = mem.read_obj(GuestAddress(SMBIOS_START)).unwrap(); diff --git a/vmm/src/config.rs b/vmm/src/config.rs index c33670953c..22ad89306f 100644 --- a/vmm/src/config.rs +++ b/vmm/src/config.rs @@ -901,8 +901,7 @@ impl PlatformConfig { let oem_strings = parser .convert::("oem_strings") .map_err(Error::ParsePlatform)? - .map(|v| v.0) - .unwrap_or_default(); + .map(|v| v.0); let iommufd = parser .convert::("iommufd") .map_err(Error::ParsePlatform)? @@ -5049,7 +5048,7 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" iommu_address_width_bits: MAX_IOMMU_ADDRESS_WIDTH_BITS, serial_number: None, uuid: None, - oem_strings: Vec::new(), + oem_strings: None, iommufd: false, vfio_p2p_dma: default_platformconfig_vfio_p2p_dma(), #[cfg(feature = "tdx")] diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs index 8e852d65ac..98f4764198 100644 --- a/vmm/src/vm.rs +++ b/vmm/src/vm.rs @@ -1944,8 +1944,11 @@ impl Vm { .unwrap() .platform .as_ref() - .map(|p| p.oem_strings.clone()) - .unwrap_or_default(); + .and_then(|p| p.oem_strings.clone()); + + let oem_strings = oem_strings + .as_deref() + .map(|strings| strings.iter().map(|s| s.as_ref()).collect::>()); let topology = self.cpu_manager.lock().unwrap().get_vcpu_topology(); @@ -1959,7 +1962,7 @@ impl Vm { rsdp_addr, serial_number.as_deref(), uuid.as_deref(), - oem_strings, + oem_strings.as_deref(), topology, ) .map_err(Error::ConfigureSystem)?; diff --git a/vmm/src/vm_config.rs b/vmm/src/vm_config.rs index f135b1e839..c1552cddea 100644 --- a/vmm/src/vm_config.rs +++ b/vmm/src/vm_config.rs @@ -134,7 +134,7 @@ pub struct PlatformConfig { #[serde(default)] pub uuid: Option, #[serde(default)] - pub oem_strings: Vec, + pub oem_strings: Option>, #[cfg(feature = "tdx")] #[serde(default)] pub tdx: bool, From 505e091aa5ec750afb8be979eb1c5d4a8415da2a Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Thu, 14 May 2026 11:04:02 +0200 Subject: [PATCH 06/15] vmm: store PCI MMIO allocators as slices The allocator lists are sized from the fixed PCI segment count. They are only indexed afterwards, so boxed slices fit the use. On-behalf-of: SAP philipp.schuster@sap.com Signed-off-by: Philipp Schuster --- vmm/src/device_manager.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index 80598d1d02..b0fd1477a5 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -712,8 +712,8 @@ pub(crate) struct AddressManager { pub(crate) mmio_bus: Arc, pub(crate) vm: Arc, device_tree: Arc>, - pci_mmio32_allocators: Vec>>, - pci_mmio64_allocators: Vec>>, + pci_mmio32_allocators: Box<[Arc>]>, + pci_mmio64_allocators: Box<[Arc>]>, } impl DeviceRelocation for AddressManager { @@ -1159,7 +1159,7 @@ fn create_mmio_allocators( num_pci_segments: u16, weights: &[u32], alignment: u64, -) -> Vec>> { +) -> Box<[Arc>]> { let total_weight: u32 = weights.iter().sum(); // Start each PCI segment mmio range on an aligned boundary @@ -1186,7 +1186,7 @@ fn create_mmio_allocators( i += weight; } - mmio_allocators + mmio_allocators.into_boxed_slice() } fn use_64bit_bar_for_virtio_device( From 5f6060736f2a864238a1f2f79aa5565adeef0c47 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Thu, 14 May 2026 11:04:10 +0200 Subject: [PATCH 07/15] vmm: store PCI segments as a slice The PCI segment list is created once from the configured count. Later code mutates entries, but does not add or remove segments. On-behalf-of: SAP philipp.schuster@sap.com Signed-off-by: Philipp Schuster --- vmm/src/device_manager.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index b0fd1477a5..d7b3d096de 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -1047,7 +1047,7 @@ pub struct DeviceManager { // Counter to keep track of the consumed device IDs. device_id_cnt: Wrapping, - pci_segments: Vec, + pci_segments: Box<[PciSegment]>, #[cfg_attr(target_arch = "aarch64", allow(dead_code))] // MSI Interrupt Manager @@ -1401,7 +1401,7 @@ impl DeviceManager { iommu_device: None, iommu_mapping: None, iommu_attached_devices: None, - pci_segments, + pci_segments: pci_segments.into_boxed_slice(), device_tree, exit_evt, reset_evt, @@ -4595,7 +4595,7 @@ impl DeviceManager { .map(|ic| ic.clone() as Arc>) } - pub(crate) fn pci_segments(&self) -> &Vec { + pub(crate) fn pci_segments(&self) -> &[PciSegment] { &self.pci_segments } From da0501ceff85c9ad9d5776ff5b65a67c9bf2768b Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Thu, 14 May 2026 11:04:19 +0200 Subject: [PATCH 08/15] vmm: store CPU affinity lists as slices Each affinity host CPU list is copied from configuration. It is only iterated afterwards, so a boxed slice is enough. On-behalf-of: SAP philipp.schuster@sap.com Signed-off-by: Philipp Schuster --- vmm/src/config.rs | 10 +++++----- vmm/src/cpu.rs | 2 +- vmm/src/vm_config.rs | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/vmm/src/config.rs b/vmm/src/config.rs index 22ad89306f..5dc34e3dc3 100644 --- a/vmm/src/config.rs +++ b/vmm/src/config.rs @@ -724,7 +724,7 @@ impl CpusConfig { v.0.iter() .map(|(e1, e2)| CpuAffinity { vcpu: *e1, - host_cpus: e2.clone(), + host_cpus: e2.clone().into_boxed_slice(), }) .collect() }); @@ -3758,16 +3758,16 @@ mod unit_tests { CpusConfig { boot_vcpus: 2, max_vcpus: 2, - affinity: Some(vec![ + affinity: Some(Box::new([ CpuAffinity { vcpu: 0, - host_cpus: vec![0, 2], + host_cpus: Box::new([0, 2]), }, CpuAffinity { vcpu: 1, - host_cpus: vec![1, 3], + host_cpus: Box::new([1, 3]), } - ]), + ])), ..Default::default() }, ); diff --git a/vmm/src/cpu.rs b/vmm/src/cpu.rs index b23ae0a413..83f000caec 100644 --- a/vmm/src/cpu.rs +++ b/vmm/src/cpu.rs @@ -752,7 +752,7 @@ pub struct CpuManager { #[cfg_attr(target_arch = "aarch64", allow(dead_code))] acpi_address: Option, proximity_domain_per_cpu: BTreeMap, - affinity: BTreeMap>, + affinity: BTreeMap>, dynamic: bool, hypervisor: Arc, #[cfg(feature = "sev_snp")] diff --git a/vmm/src/vm_config.rs b/vmm/src/vm_config.rs index c1552cddea..f6b98b9da9 100644 --- a/vmm/src/vm_config.rs +++ b/vmm/src/vm_config.rs @@ -31,7 +31,7 @@ pub(crate) trait ApplyLandlock { #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)] pub struct CpuAffinity { pub vcpu: u32, - pub host_cpus: Vec, + pub host_cpus: Box<[usize]>, } #[derive(Clone, Debug, Default, PartialEq, Eq, Deserialize, Serialize)] @@ -77,7 +77,7 @@ pub struct CpusConfig { #[serde(default = "default_cpuconfig_max_phys_bits")] pub max_phys_bits: u8, #[serde(default)] - pub affinity: Option>, + pub affinity: Option>, #[serde(default)] pub features: CpuFeatures, #[serde(default = "default_cpusconfig_nested")] From 58983bee86cf3c0a46499f96147e92b6d0e713f0 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Fri, 15 May 2026 10:26:59 +0200 Subject: [PATCH 09/15] virtio-devices, vmm: replace Vec with Box<[T]> in config structs I started by looking at all `Option>` values in config.rs and vm_config.rs, and replaced them with `Option>`. This has the advantage that one now can see at a glance if this field will ever resize during operation or not, reducing cognitive load and increasing maintainability. All fields that need the properties of a Ver or where this change was not trivial are kept intact. On-behalf-of: SAP philipp.schuster@sap.com Signed-off-by: Philipp Schuster --- virtio-devices/src/block.rs | 14 ++-- vmm/src/config.rs | 142 ++++++++++++++++++------------------ vmm/src/vm_config.rs | 24 +++--- 3 files changed, 93 insertions(+), 87 deletions(-) diff --git a/virtio-devices/src/block.rs b/virtio-devices/src/block.rs index 6946924f31..f40f1a49a9 100644 --- a/virtio-devices/src/block.rs +++ b/virtio-devices/src/block.rs @@ -155,7 +155,7 @@ struct BlockEpollHandler { disk_image: Box, disk_nsectors: Arc, interrupt_cb: Arc, - serial: Vec, + serial: Box<[u8]>, kill_evt: EventFd, pause_evt: EventFd, writeback: Arc, @@ -164,7 +164,7 @@ struct BlockEpollHandler { inflight_requests: VecDeque<(u16, Request)>, rate_limiter: Option, access_platform: Option>, - host_cpus: Option>, + host_cpus: Option>, acked_features: u64, disable_sector0_writes: bool, } @@ -723,8 +723,8 @@ pub struct Block { seccomp_action: SeccompAction, rate_limiter: Option>, exit_evt: EventFd, - serial: Vec, - queue_affinity: BTreeMap>, + serial: Box<[u8]>, + queue_affinity: BTreeMap>, disable_sector0_writes: bool, lock_granularity_choice: LockGranularityChoice, device_status: Arc, @@ -755,7 +755,7 @@ impl Block { rate_limiter: Option>, exit_evt: EventFd, state: Option, - queue_affinity: BTreeMap>, + queue_affinity: BTreeMap>, sparse: bool, disable_sector0_writes: bool, lock_granularity: LockGranularityChoice, @@ -862,7 +862,9 @@ impl Block { (disk_nsectors, avail_features, 0, config, false) }; - let serial = serial.map_or_else(|| build_serial(&disk_path), Vec::from); + let serial = serial + .map_or_else(|| build_serial(&disk_path), Vec::from) + .into_boxed_slice(); Ok(Block { common: VirtioCommon { diff --git a/vmm/src/config.rs b/vmm/src/config.rs index 5dc34e3dc3..2825f193de 100644 --- a/vmm/src/config.rs +++ b/vmm/src/config.rs @@ -901,7 +901,7 @@ impl PlatformConfig { let oem_strings = parser .convert::("oem_strings") .map_err(Error::ParsePlatform)? - .map(|v| v.0); + .map(|v| v.0.into_boxed_slice()); let iommufd = parser .convert::("iommufd") .map_err(Error::ParsePlatform)? @@ -1402,7 +1402,7 @@ impl DiskConfig { v.0.iter() .map(|(e1, e2)| VirtQueueAffinity { queue_index: *e1, - host_cpus: e2.clone(), + host_cpus: e2.clone().into_boxed_slice(), }) .collect() }); @@ -2500,7 +2500,7 @@ impl NumaConfig { let memory_zones = parser .convert::("memory_zones") .map_err(Error::ParseNuma)? - .map(|v| v.0); + .map(|v| v.0.into_boxed_slice()); let pci_segments = parser .convert::("pci_segments") .map_err(Error::ParseNuma)? @@ -3302,14 +3302,14 @@ impl VmConfig { } pub fn parse(vm_params: VmParams) -> Result { - let mut rate_limit_groups: Option> = None; + let mut rate_limit_groups: Option> = None; if let Some(rate_limit_group_list) = &vm_params.rate_limit_groups { let mut rate_limit_group_config_list = Vec::new(); for item in rate_limit_group_list.iter() { let rate_limit_group_config = RateLimiterGroupConfig::parse(item)?; rate_limit_group_config_list.push(rate_limit_group_config); } - rate_limit_groups = Some(rate_limit_group_config_list); + rate_limit_groups = Some(rate_limit_group_config_list.into_boxed_slice()); } let mut disks: Option> = None; @@ -3421,26 +3421,26 @@ impl VmConfig { vsock = Some(vsock_config); } - let mut pci_segments: Option> = None; + let mut pci_segments: Option> = None; if let Some(pci_segment_list) = &vm_params.pci_segments { let mut pci_segment_config_list = Vec::new(); for item in pci_segment_list.iter() { let pci_segment_config = PciSegmentConfig::parse(item)?; pci_segment_config_list.push(pci_segment_config); } - pci_segments = Some(pci_segment_config_list); + pci_segments = Some(pci_segment_config_list.into_boxed_slice()); } let platform = vm_params.platform.map(PlatformConfig::parse).transpose()?; - let mut numa: Option> = None; + let mut numa: Option> = None; if let Some(numa_list) = &vm_params.numa { let mut numa_config_list = Vec::new(); for item in numa_list.iter() { let numa_config = NumaConfig::parse(item)?; numa_config_list.push(numa_config); } - numa = Some(numa_config_list); + numa = Some(numa_config_list.into_boxed_slice()); } #[cfg(not(feature = "igvm"))] @@ -3478,13 +3478,14 @@ impl VmConfig { #[cfg(feature = "guest_debug")] let gdb = vm_params.gdb; - let mut landlock_rules: Option> = None; + let mut landlock_rules: Option> = None; if let Some(ll_rules) = vm_params.landlock_rules { landlock_rules = Some( ll_rules .iter() .map(|rule| LandlockConfig::parse(rule)) - .collect::>>()?, + .collect::>>()? + .into_boxed_slice(), ); } @@ -4149,24 +4150,24 @@ mod unit_tests { assert_eq!( DiskConfig::parse("path=/path/to_file,queue_affinity=[0@[1],1@[2],2@[3,4],3@[5-8]]")?, DiskConfig { - queue_affinity: Some(vec![ + queue_affinity: Some(Box::new([ VirtQueueAffinity { queue_index: 0, - host_cpus: vec![1], + host_cpus: Box::new([1]), }, VirtQueueAffinity { queue_index: 1, - host_cpus: vec![2], + host_cpus: Box::new([2]), }, VirtQueueAffinity { queue_index: 2, - host_cpus: vec![3, 4], + host_cpus: Box::new([3, 4]), }, VirtQueueAffinity { queue_index: 3, - host_cpus: vec![5, 6, 7, 8], + host_cpus: Box::new([5, 6, 7, 8]), } - ]), + ])), ..disk_fixture() } ); @@ -4684,14 +4685,14 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" memory_zones=[mem0],pci_segments=[0]"; let expected_standard = NumaConfig { guest_numa_id: 1, - cpus: Some(vec![2, 3]), - distances: Some(vec![NumaDistance { + cpus: Some(Box::new([2, 3])), + distances: Some(Box::new([NumaDistance { destination: 0, distance: 20, - }]), + }])), device_id: None, - memory_zones: Some(vec!["mem0".to_string()]), - pci_segments: Some(vec![0]), + memory_zones: Some(Box::new(["mem0".to_string()])), + pci_segments: Some(Box::new([0])), }; assert_eq!(NumaConfig::parse(standard_input)?, expected_standard); // Successful generic initiator config parse @@ -4699,13 +4700,13 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" let expected_gi = NumaConfig { guest_numa_id: 2, cpus: None, - distances: Some(vec![NumaDistance { + distances: Some(Box::new([NumaDistance { destination: 0, distance: 30, - }]), + }])), device_id: Some("vfio1".to_string()), memory_zones: None, - pci_segments: Some(vec![1]), + pci_segments: Some(Box::new([1])), }; assert_eq!(NumaConfig::parse(gi_input)?, expected_gi); Ok(()) @@ -4717,10 +4718,10 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" let config = NumaConfig { guest_numa_id: 0, cpus: None, - distances: Some(vec![NumaDistance { + distances: Some(Box::new([NumaDistance { destination: 1, distance: 20, - }]), + }])), memory_zones: None, device_id: Some("vfio0".to_string()), pci_segments: None, @@ -4748,7 +4749,7 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" // device_id and cpus specified let config = NumaConfig { guest_numa_id: 0, - cpus: Some(vec![0, 1]), + cpus: Some(Box::new([0, 1])), distances: None, device_id: Some("vfio0".to_string()), memory_zones: None, @@ -4765,7 +4766,7 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" cpus: None, distances: None, device_id: Some("vfio0".to_string()), - memory_zones: Some(vec!["mem0".to_string()]), + memory_zones: Some(Box::new(["mem0".to_string()])), pci_segments: None, }; assert!(config.validate().is_err()); @@ -4776,13 +4777,13 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" // No device_id let config = NumaConfig { guest_numa_id: 0, - cpus: Some(vec![0, 1]), - distances: Some(vec![NumaDistance { + cpus: Some(Box::new([0, 1])), + distances: Some(Box::new([NumaDistance { destination: 1, distance: 20, - }]), + }])), device_id: None, - memory_zones: Some(vec!["mem0".to_string()]), + memory_zones: Some(Box::new(["mem0".to_string()])), pci_segments: None, }; config.validate().unwrap(); @@ -5495,14 +5496,17 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" let mut still_valid_config = valid_config.clone(); still_valid_config.platform = Some(PlatformConfig { - iommu_segments: Some(vec![1, 2, 3]), + iommu_segments: Some(Box::new([1, 2, 3])), ..platform_fixture() }); still_valid_config.validate().unwrap(); let mut invalid_config = valid_config.clone(); invalid_config.platform = Some(PlatformConfig { - iommu_segments: Some(vec![MAX_NUM_PCI_SEGMENTS + 1, MAX_NUM_PCI_SEGMENTS + 2]), + iommu_segments: Some(Box::new([ + MAX_NUM_PCI_SEGMENTS + 1, + MAX_NUM_PCI_SEGMENTS + 2, + ])), ..platform_fixture() }); assert_eq!( @@ -5524,7 +5528,7 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" let mut still_valid_config = valid_config.clone(); still_valid_config.platform = Some(PlatformConfig { - iommu_segments: Some(vec![1, 2, 3]), + iommu_segments: Some(Box::new([1, 2, 3])), ..platform_fixture() }); still_valid_config.disks = Some(vec![DiskConfig { @@ -5539,7 +5543,7 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" let mut still_valid_config = valid_config.clone(); still_valid_config.platform = Some(PlatformConfig { - iommu_segments: Some(vec![1, 2, 3]), + iommu_segments: Some(Box::new([1, 2, 3])), ..platform_fixture() }); still_valid_config.net = Some(vec![NetConfig { @@ -5554,7 +5558,7 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" let mut still_valid_config = valid_config.clone(); still_valid_config.platform = Some(PlatformConfig { - iommu_segments: Some(vec![1, 2, 3]), + iommu_segments: Some(Box::new([1, 2, 3])), ..platform_fixture() }); still_valid_config.pmem = Some(vec![PmemConfig { @@ -5569,7 +5573,7 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" let mut still_valid_config = valid_config.clone(); still_valid_config.platform = Some(PlatformConfig { - iommu_segments: Some(vec![1, 2, 3]), + iommu_segments: Some(Box::new([1, 2, 3])), ..platform_fixture() }); still_valid_config.devices = Some(vec![DeviceConfig { @@ -5584,7 +5588,7 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" let mut still_valid_config = valid_config.clone(); still_valid_config.platform = Some(PlatformConfig { - iommu_segments: Some(vec![1, 2, 3]), + iommu_segments: Some(Box::new([1, 2, 3])), ..platform_fixture() }); still_valid_config.vsock = Some(VsockConfig { @@ -5600,7 +5604,7 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" let mut invalid_config = valid_config.clone(); invalid_config.platform = Some(PlatformConfig { - iommu_segments: Some(vec![1, 2, 3]), + iommu_segments: Some(Box::new([1, 2, 3])), ..platform_fixture() }); invalid_config.disks = Some(vec![DiskConfig { @@ -5618,7 +5622,7 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" let mut invalid_config = valid_config.clone(); invalid_config.platform = Some(PlatformConfig { - iommu_segments: Some(vec![1, 2, 3]), + iommu_segments: Some(Box::new([1, 2, 3])), ..platform_fixture() }); invalid_config.net = Some(vec![NetConfig { @@ -5637,7 +5641,7 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" let mut invalid_config = valid_config.clone(); invalid_config.platform = Some(PlatformConfig { num_pci_segments: MAX_NUM_PCI_SEGMENTS, - iommu_segments: Some(vec![1, 2, 3]), + iommu_segments: Some(Box::new([1, 2, 3])), ..platform_fixture() }); invalid_config.pmem = Some(vec![PmemConfig { @@ -5655,7 +5659,7 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" let mut invalid_config = valid_config.clone(); invalid_config.platform = Some(PlatformConfig { num_pci_segments: MAX_NUM_PCI_SEGMENTS, - iommu_segments: Some(vec![1, 2, 3]), + iommu_segments: Some(Box::new([1, 2, 3])), ..platform_fixture() }); invalid_config.devices = Some(vec![DeviceConfig { @@ -5672,7 +5676,7 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" let mut invalid_config = valid_config.clone(); invalid_config.platform = Some(PlatformConfig { - iommu_segments: Some(vec![1, 2, 3]), + iommu_segments: Some(Box::new([1, 2, 3])), ..platform_fixture() }); invalid_config.vsock = Some(VsockConfig { @@ -5691,7 +5695,7 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" let mut invalid_config = valid_config.clone(); invalid_config.memory.shared = true; invalid_config.platform = Some(PlatformConfig { - iommu_segments: Some(vec![1, 2, 3]), + iommu_segments: Some(Box::new([1, 2, 3])), ..platform_fixture() }); invalid_config.user_devices = Some(vec![UserDeviceConfig { @@ -5708,7 +5712,7 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" let mut invalid_config = valid_config.clone(); invalid_config.platform = Some(PlatformConfig { - iommu_segments: Some(vec![1, 2, 3]), + iommu_segments: Some(Box::new([1, 2, 3])), ..platform_fixture() }); invalid_config.vdpa = Some(vec![VdpaConfig { @@ -5726,7 +5730,7 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" let mut invalid_config = valid_config.clone(); invalid_config.memory.shared = true; invalid_config.platform = Some(PlatformConfig { - iommu_segments: Some(vec![1, 2, 3]), + iommu_segments: Some(Box::new([1, 2, 3])), ..platform_fixture() }); invalid_config.fs = Some(vec![FsConfig { @@ -5746,81 +5750,81 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" num_pci_segments: 2, ..platform_fixture() }); - invalid_config.numa = Some(vec![ + invalid_config.numa = Some(Box::new([ NumaConfig { guest_numa_id: 0, - cpus: Some(vec![0]), - pci_segments: Some(vec![1]), + cpus: Some(Box::new([0])), + pci_segments: Some(Box::new([1])), ..numa_fixture() }, NumaConfig { guest_numa_id: 1, - cpus: Some(vec![1]), - pci_segments: Some(vec![1]), + cpus: Some(Box::new([1])), + pci_segments: Some(Box::new([1])), ..numa_fixture() }, - ]); + ])); assert_eq!( invalid_config.validate(), Err(ValidationError::PciSegmentReused(1, 0, 1)) ); let mut invalid_config = valid_config.clone(); - invalid_config.pci_segments = Some(vec![PciSegmentConfig { + invalid_config.pci_segments = Some(Box::new([PciSegmentConfig { pci_segment: 0, mmio32_aperture_weight: 1, mmio64_aperture_weight: 0, - }]); + }])); assert_eq!( invalid_config.validate(), Err(ValidationError::InvalidPciSegmentApertureWeight(0)) ); let mut invalid_config = valid_config.clone(); - invalid_config.pci_segments = Some(vec![PciSegmentConfig { + invalid_config.pci_segments = Some(Box::new([PciSegmentConfig { pci_segment: 0, mmio32_aperture_weight: 0, mmio64_aperture_weight: 1, - }]); + }])); assert_eq!( invalid_config.validate(), Err(ValidationError::InvalidPciSegmentApertureWeight(0)) ); let mut invalid_config = valid_config.clone(); - invalid_config.numa = Some(vec![ + invalid_config.numa = Some(Box::new([ NumaConfig { guest_numa_id: 0, - cpus: Some(vec![0]), + cpus: Some(Box::new([0])), ..numa_fixture() }, NumaConfig { guest_numa_id: 1, - cpus: Some(vec![1]), - pci_segments: Some(vec![0]), + cpus: Some(Box::new([1])), + pci_segments: Some(Box::new([0])), ..numa_fixture() }, - ]); + ])); assert_eq!( invalid_config.validate(), Err(ValidationError::DefaultPciSegmentInvalidNode(1)) ); let mut invalid_config = valid_config.clone(); - invalid_config.numa = Some(vec![ + invalid_config.numa = Some(Box::new([ NumaConfig { guest_numa_id: 0, - cpus: Some(vec![0]), - pci_segments: Some(vec![0]), + cpus: Some(Box::new([0])), + pci_segments: Some(Box::new([0])), ..numa_fixture() }, NumaConfig { guest_numa_id: 1, - cpus: Some(vec![1]), - pci_segments: Some(vec![1]), + cpus: Some(Box::new([1])), + pci_segments: Some(Box::new([1])), ..numa_fixture() }, - ]); + ])); assert_eq!( invalid_config.validate(), Err(ValidationError::InvalidPciSegment(1)) diff --git a/vmm/src/vm_config.rs b/vmm/src/vm_config.rs index f6b98b9da9..a394619a40 100644 --- a/vmm/src/vm_config.rs +++ b/vmm/src/vm_config.rs @@ -126,7 +126,7 @@ pub struct PlatformConfig { #[serde(default = "default_platformconfig_num_pci_segments")] pub num_pci_segments: u16, #[serde(default)] - pub iommu_segments: Option>, + pub iommu_segments: Option>, #[serde(default = "default_platformconfig_iommu_address_width_bits")] pub iommu_address_width_bits: u8, #[serde(default)] @@ -134,7 +134,7 @@ pub struct PlatformConfig { #[serde(default)] pub uuid: Option, #[serde(default)] - pub oem_strings: Option>, + pub oem_strings: Option>, #[cfg(feature = "tdx")] #[serde(default)] pub tdx: bool, @@ -274,7 +274,7 @@ pub struct RateLimiterGroupConfig { #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)] pub struct VirtQueueAffinity { pub queue_index: u16, - pub host_cpus: Vec, + pub host_cpus: Box<[usize]>, } #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, Default)] @@ -318,7 +318,7 @@ pub struct DiskConfig { #[serde(default)] pub serial: Option, #[serde(default)] - pub queue_affinity: Option>, + pub queue_affinity: Option>, #[serde(default)] pub backing_files: bool, #[serde(default = "default_diskconfig_sparse")] @@ -791,13 +791,13 @@ pub struct NumaDistance { pub struct NumaConfig { pub guest_numa_id: u32, #[serde(default)] - pub cpus: Option>, + pub cpus: Option>, #[serde(default)] - pub distances: Option>, + pub distances: Option>, #[serde(default)] - pub memory_zones: Option>, + pub memory_zones: Option>, #[serde(default)] - pub pci_segments: Option>, + pub pci_segments: Option>, #[serde(default)] pub device_id: Option, } @@ -1029,7 +1029,7 @@ pub struct VmConfig { #[serde(default)] pub memory: MemoryConfig, pub payload: Option, - pub rate_limit_groups: Option>, + pub rate_limit_groups: Option>, pub disks: Option>, pub net: Option>, #[serde(default)] @@ -1056,13 +1056,13 @@ pub struct VmConfig { pub pvpanic: bool, #[serde(default)] pub iommu: bool, - pub numa: Option>, + pub numa: Option>, #[serde(default)] pub watchdog: bool, #[cfg(feature = "guest_debug")] #[serde(default)] pub gdb: bool, - pub pci_segments: Option>, + pub pci_segments: Option>, pub platform: Option, pub tpm: Option, // Preserved FDs are the ones that share the same life-time as its holding @@ -1077,7 +1077,7 @@ pub struct VmConfig { pub preserved_fds: Option>, #[serde(default)] pub landlock_enable: bool, - pub landlock_rules: Option>, + pub landlock_rules: Option>, #[cfg(feature = "ivshmem")] pub ivshmem: Option, } From 2ea6711ef59ae131dc24cf9e19783c843c490ec4 Mon Sep 17 00:00:00 2001 From: Leander Kohler Date: Mon, 9 Feb 2026 15:28:03 +0100 Subject: [PATCH 10/15] arch: x86_64: refactor SMBIOS helpers Split the System Information write into helper functions and reuse the string writer so the table layout and inputs are unchanged. On-behalf-of: SAP leander.kohler@sap.com Signed-off-by: Leander Kohler --- arch/src/x86_64/mod.rs | 11 +++- arch/src/x86_64/smbios.rs | 114 +++++++++++++++++++++++++------------- vmm/src/vm.rs | 4 -- 3 files changed, 82 insertions(+), 47 deletions(-) diff --git a/arch/src/x86_64/mod.rs b/arch/src/x86_64/mod.rs index 3e3152e129..c00107b21d 100644 --- a/arch/src/x86_64/mod.rs +++ b/arch/src/x86_64/mod.rs @@ -1261,7 +1261,7 @@ pub fn configure_system( rsdp_addr: Option, serial_number: Option<&str>, uuid: Option<&str>, - oem_strings: Option<&[&str]>, + oem_strings: Option<&[String]>, topology: Option<(u16, u16, u16, u16)>, ) -> super::Result<()> { // Write EBDA address to location where ACPICA expects to find it @@ -1269,8 +1269,13 @@ pub fn configure_system( .write_obj((layout::EBDA_START.0 >> 4) as u16, layout::EBDA_POINTER) .map_err(Error::EbdaSetup)?; - let size = smbios::setup_smbios(guest_mem, serial_number, uuid, oem_strings) - .map_err(Error::SmbiosSetup)?; + let size = smbios::setup_smbios( + guest_mem, + serial_number, + uuid, + oem_strings.unwrap_or_default(), + ) + .map_err(Error::SmbiosSetup)?; // Place the MP table after the SMIOS table aligned to 16 bytes let offset = GuestAddress(layout::SMBIOS_START).unchecked_add(size); diff --git a/arch/src/x86_64/smbios.rs b/arch/src/x86_64/smbios.rs index 6f1139888b..87c8f00d98 100644 --- a/arch/src/x86_64/smbios.rs +++ b/arch/src/x86_64/smbios.rs @@ -47,6 +47,8 @@ const OEM_STRINGS: u8 = 11; const END_OF_TABLE: u8 = 127; const PCI_SUPPORTED: u64 = 1 << 7; const IS_VIRTUAL_MACHINE: u8 = 1 << 4; +pub const DEFAULT_SYSTEM_MANUFACTURER: &str = "Cloud Hypervisor"; +pub const DEFAULT_SYSTEM_PRODUCT_NAME: &str = "cloud-hypervisor"; fn compute_checksum(v: &T) -> u8 { let v: *const T = v; @@ -59,8 +61,7 @@ fn compute_checksum(v: &T) -> u8 { (!checksum).wrapping_add(1) } -#[repr(C)] -#[repr(packed)] +#[repr(C, packed)] #[derive(Default, Copy, Clone)] struct Smbios30Entrypoint { signature: [u8; 5usize], @@ -75,8 +76,7 @@ struct Smbios30Entrypoint { physptr: u64, } -#[repr(C)] -#[repr(packed)] +#[repr(C, packed)] #[derive(Default, Copy, Clone)] struct SmbiosBiosInfo { r#type: u8, @@ -92,8 +92,7 @@ struct SmbiosBiosInfo { characteristics_ext2: u8, } -#[repr(C)] -#[repr(packed)] +#[repr(C, packed)] #[derive(Default, Copy, Clone)] struct SmbiosSysInfo { r#type: u8, @@ -109,8 +108,7 @@ struct SmbiosSysInfo { family: u8, } -#[repr(C)] -#[repr(packed)] +#[repr(C, packed)] #[derive(Default, Copy, Clone)] struct SmbiosOemStrings { r#type: u8, @@ -119,8 +117,7 @@ struct SmbiosOemStrings { count: u8, } -#[repr(C)] -#[repr(packed)] +#[repr(C, packed)] #[derive(Default, Copy, Clone)] struct SmbiosEndOfTable { r#type: u8, @@ -163,11 +160,73 @@ fn write_string( Ok(curptr) } +fn write_opt_string( + mem: &GuestMemoryMmap, + s: Option<&str>, + cur: GuestAddress, +) -> Result { + if let Some(v) = s { + write_string(mem, v, cur) + } else { + Ok(cur) + } +} + +fn write_string_terminator( + mem: &GuestMemoryMmap, + cur: GuestAddress, + has_strings: bool, +) -> Result { + // SMBIOS DSP0134 §6.1.3: if all string-reference fields are 0, follow the + // formatted section with two null bytes (empty string-set). + if has_strings { + write_and_incr(mem, 0u8, cur) + } else { + let cur = write_and_incr(mem, 0u8, cur)?; + write_and_incr(mem, 0u8, cur) + } +} + +fn write_type1_system( + mem: &GuestMemoryMmap, + curptr: &mut GuestAddress, + handle: &mut u16, + serial_number: Option<&str>, + uuid: Option<&str>, +) -> Result<()> { + *handle += 1; + + let uuid_number = uuid + .map(Uuid::parse_str) + .transpose() + .map_err(|e| Error::ParseUuid(e, uuid.unwrap().to_string()))? + .unwrap_or(Uuid::nil()); + let serial_idx = serial_number.map(|_| 3).unwrap_or_default(); + + let smbios_sysinfo = SmbiosSysInfo { + r#type: SYSTEM_INFORMATION, + length: mem::size_of::() as u8, + handle: *handle, + manufacturer: 1, // First string written in this section + product_name: 2, // Second string written in this section + serial_number: serial_idx, + uuid: uuid_number.to_bytes_le(), + ..Default::default() + }; + + *curptr = write_and_incr(mem, smbios_sysinfo, *curptr)?; + *curptr = write_string(mem, DEFAULT_SYSTEM_MANUFACTURER, *curptr)?; + *curptr = write_string(mem, DEFAULT_SYSTEM_PRODUCT_NAME, *curptr)?; + *curptr = write_opt_string(mem, serial_number, *curptr)?; + *curptr = write_and_incr(mem, 0u8, *curptr)?; + Ok(()) +} + pub fn setup_smbios( mem: &GuestMemoryMmap, serial_number: Option<&str>, uuid: Option<&str>, - oem_strings: Option<&[&str]>, + oem_strings: &[String], ) -> Result { let physptr = GuestAddress(SMBIOS_START) .checked_add(mem::size_of::() as u64) @@ -193,34 +252,9 @@ pub fn setup_smbios( curptr = write_and_incr(mem, 0u8, curptr)?; } - { - handle += 1; + write_type1_system(mem, &mut curptr, &mut handle, serial_number, uuid)?; - let uuid_number = uuid - .map(Uuid::parse_str) - .transpose() - .map_err(|e| Error::ParseUuid(e, uuid.unwrap().to_string()))? - .unwrap_or(Uuid::nil()); - let smbios_sysinfo = SmbiosSysInfo { - r#type: SYSTEM_INFORMATION, - length: mem::size_of::() as u8, - handle, - manufacturer: 1, // First string written in this section - product_name: 2, // Second string written in this section - serial_number: serial_number.map(|_| 3).unwrap_or_default(), // 3rd string - uuid: uuid_number.to_bytes_le(), // set uuid - ..Default::default() - }; - curptr = write_and_incr(mem, smbios_sysinfo, curptr)?; - curptr = write_string(mem, "Cloud Hypervisor", curptr)?; - curptr = write_string(mem, "cloud-hypervisor", curptr)?; - if let Some(serial_number) = serial_number { - curptr = write_string(mem, serial_number, curptr)?; - } - curptr = write_and_incr(mem, 0u8, curptr)?; - } - - if let Some(oem_strings) = oem_strings { + if !oem_strings.is_empty() { handle += 1; let smbios_oemstrings = SmbiosOemStrings { @@ -236,7 +270,7 @@ pub fn setup_smbios( curptr = write_string(mem, s, curptr)?; } - curptr = write_and_incr(mem, 0u8, curptr)?; + curptr = write_string_terminator(mem, curptr, true)?; } { @@ -299,7 +333,7 @@ mod unit_tests { fn entrypoint_checksum() { let mem = GuestMemoryMmap::from_ranges(&[(GuestAddress(SMBIOS_START), 4096)]).unwrap(); - setup_smbios(&mem, None, None, None).unwrap(); + setup_smbios(&mem, None, None, &[]).unwrap(); let smbios_ep: Smbios30Entrypoint = mem.read_obj(GuestAddress(SMBIOS_START)).unwrap(); diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs index 98f4764198..410d16f0cf 100644 --- a/vmm/src/vm.rs +++ b/vmm/src/vm.rs @@ -1946,10 +1946,6 @@ impl Vm { .as_ref() .and_then(|p| p.oem_strings.clone()); - let oem_strings = oem_strings - .as_deref() - .map(|strings| strings.iter().map(|s| s.as_ref()).collect::>()); - let topology = self.cpu_manager.lock().unwrap().get_vcpu_topology(); arch::configure_system( From 27edeb97428129324c5606f1d30d551765d6fd2b Mon Sep 17 00:00:00 2001 From: Leander Kohler Date: Mon, 9 Feb 2026 15:34:16 +0100 Subject: [PATCH 11/15] vmm: plumb legacy SMBIOS config Add a small SMBIOS config that carries serial_number, uuid, and OEM strings, and pass it from platform config into x86_64 setup. On-behalf-of: SAP leander.kohler@sap.com Signed-off-by: Leander Kohler --- arch/src/x86_64/mod.rs | 21 +++------------------ arch/src/x86_64/smbios.rs | 25 ++++++++++++++++++------- vmm/src/vm.rs | 24 +++--------------------- vmm/src/vm_config.rs | 15 +++++++++++++++ 4 files changed, 39 insertions(+), 46 deletions(-) diff --git a/arch/src/x86_64/mod.rs b/arch/src/x86_64/mod.rs index c00107b21d..0671e17fab 100644 --- a/arch/src/x86_64/mod.rs +++ b/arch/src/x86_64/mod.rs @@ -36,6 +36,7 @@ use linux_loader::loader::elf::start_info::{ use log::{debug, error, info, trace}; pub use msr_filter::{MAX_BITMAP_SIZE, filter_denied_msrs}; use serde::{Deserialize, Serialize}; +pub use smbios::SmbiosConfig; use thiserror::Error; use vm_memory::{ Address, Bytes, GuestAddress, GuestAddressSpace, GuestMemory, GuestMemoryAtomic, @@ -1259,9 +1260,7 @@ pub fn configure_system( _num_cpus: u32, setup_header: Option, rsdp_addr: Option, - serial_number: Option<&str>, - uuid: Option<&str>, - oem_strings: Option<&[String]>, + smbios: Option<&SmbiosConfig>, topology: Option<(u16, u16, u16, u16)>, ) -> super::Result<()> { // Write EBDA address to location where ACPICA expects to find it @@ -1269,13 +1268,7 @@ pub fn configure_system( .write_obj((layout::EBDA_START.0 >> 4) as u16, layout::EBDA_POINTER) .map_err(Error::EbdaSetup)?; - let size = smbios::setup_smbios( - guest_mem, - serial_number, - uuid, - oem_strings.unwrap_or_default(), - ) - .map_err(Error::SmbiosSetup)?; + let size = smbios::setup_smbios(guest_mem, smbios).map_err(Error::SmbiosSetup)?; // Place the MP table after the SMIOS table aligned to 16 bytes let offset = GuestAddress(layout::SMBIOS_START).unchecked_add(size); @@ -1820,8 +1813,6 @@ mod unit_tests { Some(layout::RSDP_POINTER), None, None, - None, - None, ); config_err.unwrap_err(); @@ -1844,8 +1835,6 @@ mod unit_tests { None, None, None, - None, - None, ) .unwrap(); @@ -1873,8 +1862,6 @@ mod unit_tests { None, None, None, - None, - None, ) .unwrap(); @@ -1888,8 +1875,6 @@ mod unit_tests { None, None, None, - None, - None, ) .unwrap(); } diff --git a/arch/src/x86_64/smbios.rs b/arch/src/x86_64/smbios.rs index 87c8f00d98..4cef9b5e16 100644 --- a/arch/src/x86_64/smbios.rs +++ b/arch/src/x86_64/smbios.rs @@ -50,6 +50,19 @@ const IS_VIRTUAL_MACHINE: u8 = 1 << 4; pub const DEFAULT_SYSTEM_MANUFACTURER: &str = "Cloud Hypervisor"; pub const DEFAULT_SYSTEM_PRODUCT_NAME: &str = "cloud-hypervisor"; +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct SmbiosConfig { + pub serial_number: Option, + pub uuid: Option, + pub oem_strings: Box<[String]>, +} + +impl SmbiosConfig { + pub fn is_empty(&self) -> bool { + *self == Self::default() + } +} + fn compute_checksum(v: &T) -> u8 { let v: *const T = v; // SAFETY: we are only reading the bytes within the size of the `T` reference `v`. @@ -222,12 +235,10 @@ fn write_type1_system( Ok(()) } -pub fn setup_smbios( - mem: &GuestMemoryMmap, - serial_number: Option<&str>, - uuid: Option<&str>, - oem_strings: &[String], -) -> Result { +pub fn setup_smbios(mem: &GuestMemoryMmap, smbios: Option<&SmbiosConfig>) -> Result { + let serial_number = smbios.and_then(|cfg| cfg.serial_number.as_deref()); + let uuid = smbios.and_then(|cfg| cfg.uuid.as_deref()); + let oem_strings: &[String] = smbios.map_or(&[], |cfg| &cfg.oem_strings); let physptr = GuestAddress(SMBIOS_START) .checked_add(mem::size_of::() as u64) .ok_or(Error::NotEnoughMemory)?; @@ -333,7 +344,7 @@ mod unit_tests { fn entrypoint_checksum() { let mem = GuestMemoryMmap::from_ranges(&[(GuestAddress(SMBIOS_START), 4096)]).unwrap(); - setup_smbios(&mem, None, None, &[]).unwrap(); + setup_smbios(&mem, None).unwrap(); let smbios_ep: Smbios30Entrypoint = mem.read_obj(GuestAddress(SMBIOS_START)).unwrap(); diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs index 410d16f0cf..d62173e5fa 100644 --- a/vmm/src/vm.rs +++ b/vmm/src/vm.rs @@ -1922,29 +1922,13 @@ impl Vm { let boot_vcpus = self.cpu_manager.lock().unwrap().boot_vcpus(); - let serial_number = self + let smbios = self .config .lock() .unwrap() .platform .as_ref() - .and_then(|p| p.serial_number.clone()); - - let uuid = self - .config - .lock() - .unwrap() - .platform - .as_ref() - .and_then(|p| p.uuid.clone()); - - let oem_strings = self - .config - .lock() - .unwrap() - .platform - .as_ref() - .and_then(|p| p.oem_strings.clone()); + .and_then(|p| p.smbios_config()); let topology = self.cpu_manager.lock().unwrap().get_vcpu_topology(); @@ -1956,9 +1940,7 @@ impl Vm { boot_vcpus, entry_addr.setup_header, rsdp_addr, - serial_number.as_deref(), - uuid.as_deref(), - oem_strings.as_deref(), + smbios.as_ref(), topology, ) .map_err(Error::ConfigureSystem)?; diff --git a/vmm/src/vm_config.rs b/vmm/src/vm_config.rs index a394619a40..9d265bc372 100644 --- a/vmm/src/vm_config.rs +++ b/vmm/src/vm_config.rs @@ -147,6 +147,21 @@ pub struct PlatformConfig { pub vfio_p2p_dma: bool, } +#[cfg(target_arch = "x86_64")] +impl PlatformConfig { + /// Returns `None` if no SMBIOS-relevant platform fields are set, otherwise + /// `Some` with a [`SmbiosConfig`] built from the populated fields. + pub fn smbios_config(&self) -> Option { + let smbios = arch::x86_64::SmbiosConfig { + serial_number: self.serial_number.clone(), + uuid: self.uuid.clone(), + oem_strings: self.oem_strings.clone().unwrap_or_default(), + }; + + (!smbios.is_empty()).then_some(smbios) + } +} + pub const DEFAULT_PCI_SEGMENT_APERTURE_WEIGHT: u32 = 1; fn default_pci_segment_aperture_weight() -> u32 { From c5461e8fa80f4f2287f28948209a492c964e8496 Mon Sep 17 00:00:00 2001 From: Leander Kohler Date: Mon, 9 Feb 2026 15:44:24 +0100 Subject: [PATCH 12/15] vmm: platform: add structured SMBIOS config Extend SMBIOS System Information with manufacturer, product, version, family, sku, serial, and uuid fields, add a chassis asset tag, and pass a structured SMBIOS config from --platform into arch setup. Keep OEM strings and legacy serial_number/uuid options working for compatibility. The platform option naming follows `dmidecode -s `. Fields: - system_manufacturer - system_product_name - system_version - system_family - system_serial_number - system_uuid - chassis_asset_tag On-behalf-of: SAP leander.kohler@sap.com Signed-off-by: Leander Kohler --- arch/src/x86_64/mod.rs | 2 +- arch/src/x86_64/smbios.rs | 170 ++++++++++++++++-- .../tests/common/tests_wrappers.rs | 6 +- vmm/src/api/openapi/cloud-hypervisor.yaml | 16 ++ vmm/src/config.rs | 105 +++++++++-- vmm/src/vm_config.rs | 51 +++++- 6 files changed, 312 insertions(+), 38 deletions(-) diff --git a/arch/src/x86_64/mod.rs b/arch/src/x86_64/mod.rs index 0671e17fab..b8c10d9e0e 100644 --- a/arch/src/x86_64/mod.rs +++ b/arch/src/x86_64/mod.rs @@ -36,7 +36,7 @@ use linux_loader::loader::elf::start_info::{ use log::{debug, error, info, trace}; pub use msr_filter::{MAX_BITMAP_SIZE, filter_denied_msrs}; use serde::{Deserialize, Serialize}; -pub use smbios::SmbiosConfig; +pub use smbios::{SmbiosChassisConfig, SmbiosConfig, SmbiosSystem}; use thiserror::Error; use vm_memory::{ Address, Bytes, GuestAddress, GuestAddressSpace, GuestMemory, GuestMemoryAtomic, diff --git a/arch/src/x86_64/smbios.rs b/arch/src/x86_64/smbios.rs index 4cef9b5e16..44382a0f2f 100644 --- a/arch/src/x86_64/smbios.rs +++ b/arch/src/x86_64/smbios.rs @@ -35,16 +35,24 @@ pub enum Error { /// Failure to parse uuid, uuid format may be error #[error("Failure to parse uuid: {1}")] ParseUuid(#[source] uuid::Error, String), + /// SMBIOS string index overflow (u8 limit reached). + #[error("SMBIOS string index overflow (u8 limit reached: {})", u8::MAX)] + TooManyStrings, } pub type Result = result::Result; -// Constants sourced from SMBIOS Spec 3.2.0. +// Constants sourced from SMBIOS Spec 3.9.0. const SM3_MAGIC_IDENT: &[u8; 5usize] = b"_SM3_"; const BIOS_INFORMATION: u8 = 0; const SYSTEM_INFORMATION: u8 = 1; const OEM_STRINGS: u8 = 11; +const SYSTEM_ENCLOSURE: u8 = 3; const END_OF_TABLE: u8 = 127; +const SYSTEM_WAKE_UP_TYPE_UNKNOWN: u8 = 0x02; +const CHASSIS_TYPE_UNKNOWN: u8 = 0x02; +const CHASSIS_STATE_UNKNOWN: u8 = 0x02; +const CHASSIS_SECURITY_STATUS_NONE: u8 = 0x03; const PCI_SUPPORTED: u64 = 1 << 7; const IS_VIRTUAL_MACHINE: u8 = 1 << 4; pub const DEFAULT_SYSTEM_MANUFACTURER: &str = "Cloud Hypervisor"; @@ -52,9 +60,25 @@ pub const DEFAULT_SYSTEM_PRODUCT_NAME: &str = "cloud-hypervisor"; #[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct SmbiosConfig { + pub system: Option, + pub chassis: Option, + pub oem_strings: Box<[String]>, +} + +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct SmbiosSystem { + pub manufacturer: Option, + pub product_name: Option, + pub version: Option, pub serial_number: Option, pub uuid: Option, - pub oem_strings: Box<[String]>, + pub sku_number: Option, + pub family: Option, +} + +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct SmbiosChassisConfig { + pub asset_tag: Option, } impl SmbiosConfig { @@ -130,6 +154,33 @@ struct SmbiosOemStrings { count: u8, } +/// SMBIOS Chassis Table (Type 3) as defined in DMTF SMBIOS 3.9.0: +/// https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.9.0.pdf +/// Note: trailing fields are omitted, so this structure is not complete. +#[repr(C, packed)] +#[derive(Default, Copy, Clone)] +struct SmbiosChassis { + r#type: u8, + length: u8, + handle: u16, + manufacturer: u8, + chassis_type: u8, + version: u8, + serial_number: u8, + asset_tag: u8, + bootup_state: u8, + power_supply_state: u8, + thermal_state: u8, + security_status: u8, + oem_defined: u32, + height: u8, + number_of_power_cords: u8, + contained_element_count: u8, + contained_element_record_length: u8, + // followed by contained element records (optional, variable-length) + // followed by sku_number: u8, rack_type: u8, rack_height: u8 +} + #[repr(C, packed)] #[derive(Default, Copy, Clone)] struct SmbiosEndOfTable { @@ -147,6 +198,8 @@ unsafe impl ByteValued for SmbiosSysInfo {} // SAFETY: data structure only contain a series of integers unsafe impl ByteValued for SmbiosOemStrings {} // SAFETY: data structure only contain a series of integers +unsafe impl ByteValued for SmbiosChassis {} +// SAFETY: data structure only contain a series of integers unsafe impl ByteValued for SmbiosEndOfTable {} fn write_and_incr( @@ -200,44 +253,125 @@ fn write_string_terminator( } } +/// Allocate the next string index for an SMBIOS string-set. +/// +/// Per SMBIOS DSP0134, index `0` means "no string", so valid indices run from +/// `1` to `255`. Returns `0` when `present` is `false`. Otherwise returns the +/// current value of `*next` and advances it by one. Fails with +/// [`Error::TooManyStrings`] once all 255 indices have been used: `next` +/// starts at `1`, so it can only be `0` here after wrapping past `255`. +fn alloc_index(next: &mut u8, present: bool) -> Result { + if !present { + return Ok(0); + } + + let idx = *next; + if idx == 0 { + return Err(Error::TooManyStrings); + } + + *next = next.wrapping_add(1); + Ok(idx) +} + fn write_type1_system( mem: &GuestMemoryMmap, curptr: &mut GuestAddress, handle: &mut u16, - serial_number: Option<&str>, - uuid: Option<&str>, + system: Option<&SmbiosSystem>, ) -> Result<()> { *handle += 1; + let manufacturer = system + .and_then(|s| s.manufacturer.as_deref()) + .unwrap_or(DEFAULT_SYSTEM_MANUFACTURER); + let product = system + .and_then(|s| s.product_name.as_deref()) + .unwrap_or(DEFAULT_SYSTEM_PRODUCT_NAME); + let version = system.and_then(|s| s.version.as_deref()); + let serial = system.and_then(|s| s.serial_number.as_deref()); + let uuid = system.and_then(|s| s.uuid.as_deref()); + let sku = system.and_then(|s| s.sku_number.as_deref()); + let family = system.and_then(|s| s.family.as_deref()); + let uuid_number = uuid .map(Uuid::parse_str) .transpose() .map_err(|e| Error::ParseUuid(e, uuid.unwrap().to_string()))? .unwrap_or(Uuid::nil()); - let serial_idx = serial_number.map(|_| 3).unwrap_or_default(); - let smbios_sysinfo = SmbiosSysInfo { + let mut next = 1u8; + let manufacturer_idx = alloc_index(&mut next, true)?; + let product_idx = alloc_index(&mut next, true)?; + let version_idx = alloc_index(&mut next, version.is_some())?; + let serial_idx = alloc_index(&mut next, serial.is_some())?; + let sku_idx = alloc_index(&mut next, sku.is_some())?; + let family_idx = alloc_index(&mut next, family.is_some())?; + + let sys = SmbiosSysInfo { r#type: SYSTEM_INFORMATION, length: mem::size_of::() as u8, handle: *handle, - manufacturer: 1, // First string written in this section - product_name: 2, // Second string written in this section + manufacturer: manufacturer_idx, + product_name: product_idx, + version: version_idx, serial_number: serial_idx, uuid: uuid_number.to_bytes_le(), - ..Default::default() + wake_up_type: SYSTEM_WAKE_UP_TYPE_UNKNOWN, + sku: sku_idx, + family: family_idx, }; - *curptr = write_and_incr(mem, smbios_sysinfo, *curptr)?; - *curptr = write_string(mem, DEFAULT_SYSTEM_MANUFACTURER, *curptr)?; - *curptr = write_string(mem, DEFAULT_SYSTEM_PRODUCT_NAME, *curptr)?; - *curptr = write_opt_string(mem, serial_number, *curptr)?; + *curptr = write_and_incr(mem, sys, *curptr)?; + *curptr = write_string(mem, manufacturer, *curptr)?; + *curptr = write_string(mem, product, *curptr)?; + *curptr = write_opt_string(mem, version, *curptr)?; + *curptr = write_opt_string(mem, serial, *curptr)?; + *curptr = write_opt_string(mem, sku, *curptr)?; + *curptr = write_opt_string(mem, family, *curptr)?; *curptr = write_and_incr(mem, 0u8, *curptr)?; Ok(()) } +fn write_type3_chassis( + mem: &GuestMemoryMmap, + curptr: &mut GuestAddress, + handle: &mut u16, + chassis: &SmbiosChassisConfig, +) -> Result<()> { + *handle += 1; + + let asset_tag = chassis.asset_tag.as_deref(); + let mut next = 1u8; + let asset_idx = alloc_index(&mut next, asset_tag.is_some())?; + + let ch = SmbiosChassis { + r#type: SYSTEM_ENCLOSURE, + length: mem::size_of::() as u8, + handle: *handle, + manufacturer: 0, + chassis_type: CHASSIS_TYPE_UNKNOWN, + version: 0, + serial_number: 0, + asset_tag: asset_idx, + bootup_state: CHASSIS_STATE_UNKNOWN, + power_supply_state: CHASSIS_STATE_UNKNOWN, + thermal_state: CHASSIS_STATE_UNKNOWN, + security_status: CHASSIS_SECURITY_STATUS_NONE, + contained_element_count: 0, + contained_element_record_length: 0, + ..Default::default() + }; + + *curptr = write_and_incr(mem, ch, *curptr)?; + *curptr = write_opt_string(mem, asset_tag, *curptr)?; + *curptr = write_string_terminator(mem, *curptr, asset_tag.is_some())?; + Ok(()) +} + pub fn setup_smbios(mem: &GuestMemoryMmap, smbios: Option<&SmbiosConfig>) -> Result { - let serial_number = smbios.and_then(|cfg| cfg.serial_number.as_deref()); - let uuid = smbios.and_then(|cfg| cfg.uuid.as_deref()); + let system = smbios.and_then(|cfg| cfg.system.as_ref()); + let chassis = smbios.and_then(|cfg| cfg.chassis.as_ref()); let oem_strings: &[String] = smbios.map_or(&[], |cfg| &cfg.oem_strings); let physptr = GuestAddress(SMBIOS_START) .checked_add(mem::size_of::() as u64) @@ -263,7 +397,11 @@ pub fn setup_smbios(mem: &GuestMemoryMmap, smbios: Option<&SmbiosConfig>) -> Res curptr = write_and_incr(mem, 0u8, curptr)?; } - write_type1_system(mem, &mut curptr, &mut handle, serial_number, uuid)?; + write_type1_system(mem, &mut curptr, &mut handle, system)?; + + if let Some(chassis) = chassis { + write_type3_chassis(mem, &mut curptr, &mut handle, chassis)?; + } if !oem_strings.is_empty() { handle += 1; diff --git a/cloud-hypervisor/tests/common/tests_wrappers.rs b/cloud-hypervisor/tests/common/tests_wrappers.rs index 693702b552..27e4c1f272 100644 --- a/cloud-hypervisor/tests/common/tests_wrappers.rs +++ b/cloud-hypervisor/tests/common/tests_wrappers.rs @@ -2219,7 +2219,7 @@ pub(crate) fn _test_dmi_serial_number(guest: &Guest) { let mut child = GuestCommand::new(guest) .default_cpus() .default_memory() - .default_kernel_cmdline_with_platform(Some("serial_number=a=b;c=d")) + .default_kernel_cmdline_with_platform(Some("system_serial_number=a=b;c=d")) .default_disks() .default_net() .capture_output() @@ -2248,7 +2248,9 @@ pub(crate) fn _test_dmi_uuid(guest: &Guest) { let mut child = GuestCommand::new(guest) .default_cpus() .default_memory() - .default_kernel_cmdline_with_platform(Some("uuid=1e8aa28a-435d-4027-87f4-40dceff1fa0a")) + .default_kernel_cmdline_with_platform(Some( + "system_uuid=1e8aa28a-435d-4027-87f4-40dceff1fa0a", + )) .default_disks() .default_net() .capture_output() diff --git a/vmm/src/api/openapi/cloud-hypervisor.yaml b/vmm/src/api/openapi/cloud-hypervisor.yaml index d3c06ebf39..5ef14737fb 100644 --- a/vmm/src/api/openapi/cloud-hypervisor.yaml +++ b/vmm/src/api/openapi/cloud-hypervisor.yaml @@ -794,14 +794,30 @@ components: iommu_address_width: type: integer format: uint8 + system_serial_number: + type: string serial_number: type: string + system_uuid: + type: string uuid: type: string oem_strings: type: array items: type: string + system_manufacturer: + type: string + system_product_name: + type: string + system_version: + type: string + system_family: + type: string + system_sku_number: + type: string + chassis_asset_tag: + type: string tdx: type: boolean default: false diff --git a/vmm/src/config.rs b/vmm/src/config.rs index 2825f193de..fa57aaa48f 100644 --- a/vmm/src/config.rs +++ b/vmm/src/config.rs @@ -844,9 +844,11 @@ impl PlatformConfig { static SYNTAX: LazyLock = LazyLock::new(|| { let mut syntax = "Platform configuration parameters \ \"num_pci_segments=,iommu_segments=,\ - iommu_address_width=,serial_number=,\ - uuid=,oem_strings=,iommufd=on|off,\ - vfio_p2p_dma=on|off" + iommu_address_width=,iommufd=on|off,vfio_p2p_dma=on|off,system_manufacturer=,\ + system_product_name=,system_version=,\ + system_serial_number=,system_uuid=,\ + system_sku_number=,system_family=,\ + oem_strings=,chassis_asset_tag=" .to_string(); if cfg!(feature = "tdx") { @@ -866,6 +868,46 @@ impl PlatformConfig { } pub fn parse(platform: &str) -> Result { + struct StringField { + key: &'static str, + apply: fn(&mut PlatformConfig, String), + } + + const SMBIOS_STRING_FIELDS: &[StringField] = &[ + StringField { + key: "system_manufacturer", + apply: |config, value| config.system_manufacturer = Some(value), + }, + StringField { + key: "system_product_name", + apply: |config, value| config.system_product_name = Some(value), + }, + StringField { + key: "system_version", + apply: |config, value| config.system_version = Some(value), + }, + StringField { + key: "system_serial_number", + apply: |config, value| config.system_serial_number = Some(value), + }, + StringField { + key: "system_uuid", + apply: |config, value| config.system_uuid = Some(value), + }, + StringField { + key: "system_sku_number", + apply: |config, value| config.system_sku_number = Some(value), + }, + StringField { + key: "system_family", + apply: |config, value| config.system_family = Some(value), + }, + StringField { + key: "chassis_asset_tag", + apply: |config, value| config.chassis_asset_tag = Some(value), + }, + ]; + let mut parser = OptionParser::new(); parser .add("num_pci_segments") @@ -876,6 +918,9 @@ impl PlatformConfig { .add("oem_strings") .add("iommufd") .add("vfio_p2p_dma"); + for field in SMBIOS_STRING_FIELDS { + parser.add(field.key); + } #[cfg(feature = "tdx")] parser.add("tdx"); #[cfg(feature = "sev_snp")] @@ -894,10 +939,6 @@ impl PlatformConfig { .convert("iommu_address_width") .map_err(Error::ParsePlatform)? .unwrap_or(MAX_IOMMU_ADDRESS_WIDTH_BITS); - let serial_number = parser - .convert("serial_number") - .map_err(Error::ParsePlatform)?; - let uuid = parser.convert("uuid").map_err(Error::ParsePlatform)?; let oem_strings = parser .convert::("oem_strings") .map_err(Error::ParsePlatform)? @@ -924,20 +965,50 @@ impl PlatformConfig { .map_err(Error::ParsePlatform)? .unwrap_or(Toggle(false)) .0; - Ok(PlatformConfig { + + let mut platform_config = PlatformConfig { num_pci_segments, iommu_segments, iommu_address_width_bits, - serial_number, - uuid, + system_serial_number: None, + system_uuid: None, oem_strings, + system_manufacturer: None, + system_product_name: None, + system_version: None, + system_family: None, + system_sku_number: None, + chassis_asset_tag: None, iommufd, - vfio_p2p_dma, #[cfg(feature = "tdx")] tdx, #[cfg(feature = "sev_snp")] sev_snp, - }) + vfio_p2p_dma, + }; + + for field in SMBIOS_STRING_FIELDS { + if let Some(value) = parser + .convert::(field.key) + .map_err(Error::ParsePlatform)? + { + (field.apply)(&mut platform_config, value); + } + } + + let legacy_serial_number = parser + .convert::("serial_number") + .map_err(Error::ParsePlatform)?; + platform_config.system_serial_number = platform_config + .system_serial_number + .or(legacy_serial_number); + + let legacy_uuid = parser + .convert::("uuid") + .map_err(Error::ParsePlatform)?; + platform_config.system_uuid = platform_config.system_uuid.or(legacy_uuid); + + Ok(platform_config) } pub fn validate(&self) -> ValidationResult<()> { @@ -5047,11 +5118,17 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" num_pci_segments: MAX_NUM_PCI_SEGMENTS, iommu_segments: None, iommu_address_width_bits: MAX_IOMMU_ADDRESS_WIDTH_BITS, - serial_number: None, - uuid: None, + system_serial_number: None, + system_uuid: None, oem_strings: None, iommufd: false, vfio_p2p_dma: default_platformconfig_vfio_p2p_dma(), + system_manufacturer: None, + system_product_name: None, + system_version: None, + system_family: None, + system_sku_number: None, + chassis_asset_tag: None, #[cfg(feature = "tdx")] tdx: false, #[cfg(feature = "sev_snp")] diff --git a/vmm/src/vm_config.rs b/vmm/src/vm_config.rs index 9d265bc372..12ade3e9e0 100644 --- a/vmm/src/vm_config.rs +++ b/vmm/src/vm_config.rs @@ -129,12 +129,24 @@ pub struct PlatformConfig { pub iommu_segments: Option>, #[serde(default = "default_platformconfig_iommu_address_width_bits")] pub iommu_address_width_bits: u8, + #[serde(default, alias = "serial_number")] + pub system_serial_number: Option, + #[serde(default, alias = "uuid")] + pub system_uuid: Option, #[serde(default)] - pub serial_number: Option, + pub oem_strings: Option>, #[serde(default)] - pub uuid: Option, + pub system_manufacturer: Option, #[serde(default)] - pub oem_strings: Option>, + pub system_product_name: Option, + #[serde(default)] + pub system_version: Option, + #[serde(default)] + pub system_family: Option, + #[serde(default)] + pub system_sku_number: Option, + #[serde(default)] + pub chassis_asset_tag: Option, #[cfg(feature = "tdx")] #[serde(default)] pub tdx: bool, @@ -152,9 +164,38 @@ impl PlatformConfig { /// Returns `None` if no SMBIOS-relevant platform fields are set, otherwise /// `Some` with a [`SmbiosConfig`] built from the populated fields. pub fn smbios_config(&self) -> Option { + let has_system = [ + &self.system_serial_number, + &self.system_uuid, + &self.system_manufacturer, + &self.system_product_name, + &self.system_version, + &self.system_family, + &self.system_sku_number, + ] + .iter() + .any(|v| v.is_some()); + + let system = has_system.then_some(arch::x86_64::SmbiosSystem { + manufacturer: self.system_manufacturer.clone(), + product_name: self.system_product_name.clone(), + version: self.system_version.clone(), + serial_number: self.system_serial_number.clone(), + uuid: self.system_uuid.clone(), + sku_number: self.system_sku_number.clone(), + family: self.system_family.clone(), + }); + + let chassis = + self.chassis_asset_tag + .clone() + .map(|asset_tag| arch::x86_64::SmbiosChassisConfig { + asset_tag: Some(asset_tag), + }); + let smbios = arch::x86_64::SmbiosConfig { - serial_number: self.serial_number.clone(), - uuid: self.uuid.clone(), + system, + chassis, oem_strings: self.oem_strings.clone().unwrap_or_default(), }; From 923ecd7127f4a8a5008187aebda8b8e74c1e387d Mon Sep 17 00:00:00 2001 From: Leander Kohler Date: Mon, 9 Feb 2026 11:53:00 +0100 Subject: [PATCH 13/15] vmm: deprecate legacy SMBIOS keys in API and CLI Mark serial_number/uuid as deprecated in the OpenAPI schema and emit warnings when those legacy --platform keys are used, while continuing to accept them for compatibility. On-behalf-of: SAP leander.kohler@sap.com Signed-off-by: Leander Kohler --- vmm/src/api/openapi/cloud-hypervisor.yaml | 2 ++ vmm/src/config.rs | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/vmm/src/api/openapi/cloud-hypervisor.yaml b/vmm/src/api/openapi/cloud-hypervisor.yaml index 5ef14737fb..fd5ccec531 100644 --- a/vmm/src/api/openapi/cloud-hypervisor.yaml +++ b/vmm/src/api/openapi/cloud-hypervisor.yaml @@ -798,10 +798,12 @@ components: type: string serial_number: type: string + deprecated: true system_uuid: type: string uuid: type: string + deprecated: true oem_strings: type: array items: diff --git a/vmm/src/config.rs b/vmm/src/config.rs index fa57aaa48f..acc3b0000b 100644 --- a/vmm/src/config.rs +++ b/vmm/src/config.rs @@ -999,6 +999,9 @@ impl PlatformConfig { let legacy_serial_number = parser .convert::("serial_number") .map_err(Error::ParsePlatform)?; + if legacy_serial_number.is_some() { + warn!("'serial_number' in --platform is deprecated; use 'system_serial_number'."); + } platform_config.system_serial_number = platform_config .system_serial_number .or(legacy_serial_number); @@ -1006,6 +1009,9 @@ impl PlatformConfig { let legacy_uuid = parser .convert::("uuid") .map_err(Error::ParsePlatform)?; + if legacy_uuid.is_some() { + warn!("'uuid' in --platform is deprecated; use 'system_uuid'."); + } platform_config.system_uuid = platform_config.system_uuid.or(legacy_uuid); Ok(platform_config) From 7dfb0d68aa00d8973ab5d3f941a99e775c267d76 Mon Sep 17 00:00:00 2001 From: Leander Kohler Date: Tue, 10 Feb 2026 13:18:11 +0100 Subject: [PATCH 14/15] arch: smbios: add tests for table serialization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add unit tests that walk the SMBIOS binary layout in guest memory and verify structure ordering, string-set encoding, and error paths. Tests added: - smbios_chassis_empty_string_set_has_double_null: verify that a chassis with no strings emits the double-NUL terminator required by SMBIOS DSP0134 §6.1.3. - smbios_chassis_oem_strings_layout: verify the full chain (BIOS → System → Chassis → OEM → End) when a chassis asset tag and OEM strings are configured. - smbios_strings_terminators_default: verify the default table chain (BIOS → System → End) and check that string indices and string-set contents match for both structures. - smbios_strings_too_many: exercise alloc_index up to the u8 limit (255 strings) and verify the 256th is rejected. - smbios_uuid_invalid_rejected: ensure a malformed UUID string is rejected with Error::ParseUuid. - smbios_uuid_written_le: ensure the UUID is stored in little-endian byte order as required by SMBIOS Spec 7.2.1. - smbios_write_fails_with_too_small_memory: verify that setup_smbios fails with Error::WriteData when guest memory is too small to hold anything beyond the entry point. All tests also succeed when run with miri: cargo +nightly miri test -p arch smbios On-behalf-of: SAP leander.kohler@sap.com Signed-off-by: Leander Kohler --- arch/src/x86_64/smbios.rs | 224 +++++++++++++++++++++++++++++++++++++- 1 file changed, 221 insertions(+), 3 deletions(-) diff --git a/arch/src/x86_64/smbios.rs b/arch/src/x86_64/smbios.rs index 44382a0f2f..02965f4685 100644 --- a/arch/src/x86_64/smbios.rs +++ b/arch/src/x86_64/smbios.rs @@ -459,8 +459,55 @@ pub fn setup_smbios(mem: &GuestMemoryMmap, smbios: Option<&SmbiosConfig>) -> Res mod unit_tests { use super::*; + /// Collects all strings after a SMBIOS structure, stopping at the double-NUL terminator and returns next addr. + fn read_string_set(mem: &GuestMemoryMmap, addr: GuestAddress) -> (Vec, GuestAddress) { + let mut cur = addr; + let read_byte = |addr: GuestAddress| -> u8 { mem.read_obj(addr).unwrap() }; + + // SMBIOS string-set: NUL-terminated strings, terminated by an extra NUL. + // Empty string-set is exactly "\0\0". + if read_byte(cur) == 0 { + let next = cur.checked_add(1).unwrap(); + assert_eq!(read_byte(next), 0); + return (Vec::new(), next.checked_add(1).unwrap()); + } + + let mut strings = Vec::new(); + loop { + let mut bytes = Vec::new(); + loop { + let b = read_byte(cur); + cur = cur.checked_add(1).unwrap(); + if b == 0 { + break; + } + bytes.push(b); + } + strings.push(String::from_utf8(bytes).unwrap()); + + // If the next byte is NUL, that's the extra terminator. + if read_byte(cur) == 0 { + cur = cur.checked_add(1).unwrap(); + break; + } + } + + (strings, cur) + } + + #[test] + fn entrypoint_checksum() { + let mem = GuestMemoryMmap::from_ranges(&[(GuestAddress(SMBIOS_START), 4096)]).unwrap(); + + setup_smbios(&mem, None).unwrap(); + + let smbios_ep: Smbios30Entrypoint = mem.read_obj(GuestAddress(SMBIOS_START)).unwrap(); + + assert_eq!(compute_checksum(&smbios_ep), 0); + } + #[test] - fn struct_size() { + fn entrypoint_struct_size() { assert_eq!( mem::size_of::(), 0x18usize, @@ -479,13 +526,184 @@ mod unit_tests { } #[test] - fn entrypoint_checksum() { + fn smbios_chassis_empty_string_set_has_double_null() { + let mem = GuestMemoryMmap::from_ranges(&[(GuestAddress(SMBIOS_START), 4096)]).unwrap(); + let smbios = SmbiosConfig { + chassis: Some(SmbiosChassisConfig::default()), + ..Default::default() + }; + + setup_smbios(&mem, Some(&smbios)).unwrap(); + + let smbios_ep: Smbios30Entrypoint = mem.read_obj(GuestAddress(SMBIOS_START)).unwrap(); + let mut cur = GuestAddress(smbios_ep.physptr); + + let bios: SmbiosBiosInfo = mem.read_obj(cur).unwrap(); + cur = cur.checked_add(bios.length as u64).unwrap(); + let (_, next) = read_string_set(&mem, cur); + cur = next; + + let sys: SmbiosSysInfo = mem.read_obj(cur).unwrap(); + cur = cur.checked_add(sys.length as u64).unwrap(); + let (_, next) = read_string_set(&mem, cur); + cur = next; + + let chassis: SmbiosChassis = mem.read_obj(cur).unwrap(); + cur = cur.checked_add(chassis.length as u64).unwrap(); + // SMBIOS DSP0134 §6.1.3: empty string-set ends with double NUL. + let b0: u8 = mem.read_obj(cur).unwrap(); + let b1: u8 = mem.read_obj(cur.checked_add(1).unwrap()).unwrap(); + assert_eq!(b0, 0); + assert_eq!(b1, 0); + cur = cur.checked_add(2).unwrap(); + + let end: SmbiosEndOfTable = mem.read_obj(cur).unwrap(); + assert_eq!(end.r#type, END_OF_TABLE); + } + + #[test] + fn smbios_chassis_oem_strings_layout() { + let mem = GuestMemoryMmap::from_ranges(&[(GuestAddress(SMBIOS_START), 4096)]).unwrap(); + + let smbios = SmbiosConfig { + chassis: Some(SmbiosChassisConfig { + asset_tag: Some("rack1".to_string()), + }), + oem_strings: ["o1".to_string(), "o2".to_string()].into(), + ..Default::default() + }; + + setup_smbios(&mem, Some(&smbios)).unwrap(); + + let smbios_ep: Smbios30Entrypoint = mem.read_obj(GuestAddress(SMBIOS_START)).unwrap(); + let mut cur = GuestAddress(smbios_ep.physptr); + + let bios: SmbiosBiosInfo = mem.read_obj(cur).unwrap(); + cur = cur.checked_add(bios.length as u64).unwrap(); + let (_, next) = read_string_set(&mem, cur); + cur = next; + + let sys: SmbiosSysInfo = mem.read_obj(cur).unwrap(); + cur = cur.checked_add(sys.length as u64).unwrap(); + let (_, next) = read_string_set(&mem, cur); + cur = next; + + let chassis: SmbiosChassis = mem.read_obj(cur).unwrap(); + assert_eq!(chassis.r#type, SYSTEM_ENCLOSURE); + assert_eq!(chassis.asset_tag, 1); + cur = cur.checked_add(chassis.length as u64).unwrap(); + let (chassis_strings, next) = read_string_set(&mem, cur); + assert_eq!(chassis_strings, vec!["rack1"]); + cur = next; + + let oem: SmbiosOemStrings = mem.read_obj(cur).unwrap(); + assert_eq!(oem.r#type, OEM_STRINGS); + assert_eq!(oem.count, 2); + cur = cur.checked_add(oem.length as u64).unwrap(); + let (oem_strings, next) = read_string_set(&mem, cur); + assert_eq!(oem_strings, vec!["o1", "o2"]); + cur = next; + + let end: SmbiosEndOfTable = mem.read_obj(cur).unwrap(); + assert_eq!(end.r#type, END_OF_TABLE); + } + + #[test] + fn smbios_strings_terminators_default() { let mem = GuestMemoryMmap::from_ranges(&[(GuestAddress(SMBIOS_START), 4096)]).unwrap(); setup_smbios(&mem, None).unwrap(); let smbios_ep: Smbios30Entrypoint = mem.read_obj(GuestAddress(SMBIOS_START)).unwrap(); + let mut cur = GuestAddress(smbios_ep.physptr); + + let bios: SmbiosBiosInfo = mem.read_obj(cur).unwrap(); + assert_eq!(bios.r#type, BIOS_INFORMATION); + cur = cur.checked_add(bios.length as u64).unwrap(); + let (bios_strings, next) = read_string_set(&mem, cur); + assert_eq!(bios_strings, vec!["cloud-hypervisor", "0"]); + cur = next; + + let sys: SmbiosSysInfo = mem.read_obj(cur).unwrap(); + assert_eq!(sys.r#type, SYSTEM_INFORMATION); + assert_eq!(sys.manufacturer, 1); + assert_eq!(sys.product_name, 2); + assert_eq!(sys.version, 0); + assert_eq!(sys.serial_number, 0); + assert_eq!(sys.sku, 0); + assert_eq!(sys.family, 0); + cur = cur.checked_add(sys.length as u64).unwrap(); + let (sys_strings, next) = read_string_set(&mem, cur); + assert_eq!( + sys_strings, + vec![DEFAULT_SYSTEM_MANUFACTURER, DEFAULT_SYSTEM_PRODUCT_NAME] + ); + cur = next; - assert_eq!(compute_checksum(&smbios_ep), 0); + let end: SmbiosEndOfTable = mem.read_obj(cur).unwrap(); + assert_eq!(end.r#type, END_OF_TABLE); + } + + #[test] + fn smbios_strings_too_many() { + let mut next = 1u8; + for _ in 0..255 { + alloc_index(&mut next, true).unwrap(); + } + let err = alloc_index(&mut next, true).unwrap_err(); + assert!(matches!(err, Error::TooManyStrings)); + } + + #[test] + fn smbios_uuid_invalid_rejected() { + let mem = GuestMemoryMmap::from_ranges(&[(GuestAddress(SMBIOS_START), 4096)]).unwrap(); + let smbios = SmbiosConfig { + system: Some(SmbiosSystem { + uuid: Some("not-a-uuid".to_string()), + ..Default::default() + }), + ..Default::default() + }; + + let err = setup_smbios(&mem, Some(&smbios)).unwrap_err(); + assert!(matches!(err, Error::ParseUuid(_, _))); + } + + #[test] + fn smbios_uuid_written_le() { + let mem = GuestMemoryMmap::from_ranges(&[(GuestAddress(SMBIOS_START), 4096)]).unwrap(); + let uuid_str = "00112233-4455-6677-8899-aabbccddeeff"; + let smbios = SmbiosConfig { + system: Some(SmbiosSystem { + uuid: Some(uuid_str.to_string()), + ..Default::default() + }), + ..Default::default() + }; + + setup_smbios(&mem, Some(&smbios)).unwrap(); + + let smbios_ep: Smbios30Entrypoint = mem.read_obj(GuestAddress(SMBIOS_START)).unwrap(); + let mut cur = GuestAddress(smbios_ep.physptr); + + let bios: SmbiosBiosInfo = mem.read_obj(cur).unwrap(); + cur = cur.checked_add(bios.length as u64).unwrap(); + let (_, next) = read_string_set(&mem, cur); + cur = next; + + let sys: SmbiosSysInfo = mem.read_obj(cur).unwrap(); + assert_eq!(sys.uuid, Uuid::parse_str(uuid_str).unwrap().to_bytes_le()); + } + + #[test] + fn smbios_write_fails_with_too_small_memory() { + let mem = GuestMemoryMmap::from_ranges(&[( + GuestAddress(SMBIOS_START), + mem::size_of::(), + )]) + .unwrap(); + + let err = setup_smbios(&mem, None).unwrap_err(); + assert!(matches!(err, Error::WriteData)); } } From 953569335269370a71ed511a6fb7d59f725cf82d Mon Sep 17 00:00:00 2001 From: Leander Kohler Date: Wed, 27 May 2026 12:12:44 +0200 Subject: [PATCH 15/15] tests: cover new SMBIOS platform fields The structured SMBIOS platform config introduced six new keys (system_manufacturer, system_product_name, system_version, system_family, system_sku_number, chassis_asset_tag), but integration coverage only existed for serial_number, uuid, and oem_strings. Add _test_dmi_system_and_chassis, which boots a guest with all six keys set and checks each value via `dmidecode -s` using the same leaf name as the CLI key. Execute it in both the regular and SEV-SNP integration suites. On-behalf-of: SAP leander.kohler@sap.com Signed-off-by: Leander Kohler --- .../tests/common/tests_wrappers.rs | 48 +++++++++++++++++++ cloud-hypervisor/tests/integration.rs | 7 +++ cloud-hypervisor/tests/integration_cvm.rs | 6 +++ 3 files changed, 61 insertions(+) diff --git a/cloud-hypervisor/tests/common/tests_wrappers.rs b/cloud-hypervisor/tests/common/tests_wrappers.rs index 27e4c1f272..a236db3d5a 100644 --- a/cloud-hypervisor/tests/common/tests_wrappers.rs +++ b/cloud-hypervisor/tests/common/tests_wrappers.rs @@ -2325,6 +2325,54 @@ pub(crate) fn _test_dmi_oem_strings(guest: &Guest) { handle_child_output(r, &output); } +#[cfg(target_arch = "x86_64")] +pub(crate) fn _test_dmi_system_and_chassis(guest: &Guest) { + let fields = [ + ("system_manufacturer", "system-manufacturer", "Manufacturer"), + ("system_product_name", "system-product-name", "ProductName"), + ("system_version", "system-version", "Version"), + ("system_family", "system-family", "Family"), + ("system_sku_number", "system-sku-number", "SkuNumber"), + ("chassis_asset_tag", "chassis-asset-tag", "AssetTag"), + ]; + + let platform = fields + .iter() + .map(|(key, _, value)| format!("{key}={value}")) + .collect::>() + .join(","); + + let mut child = GuestCommand::new(guest) + .default_cpus() + .default_memory() + .default_kernel_cmdline_with_platform(Some(&platform)) + .default_disks() + .default_net() + .capture_output() + .spawn() + .unwrap(); + + let r = std::panic::catch_unwind(|| { + guest.wait_vm_boot().unwrap(); + + for (_, dmidecode_field, expected) in fields { + assert_eq!( + guest + .ssh_command(&format!("sudo dmidecode -s {dmidecode_field}")) + .unwrap() + .trim(), + expected, + "DMI field {dmidecode_field} mismatch" + ); + } + }); + + kill_child(&mut child); + let output = child.wait_with_output().unwrap(); + + handle_child_output(r, &output); +} + pub(crate) fn _test_serial_off(guest: &Guest) { let mut child = GuestCommand::new(guest) .default_cpus() diff --git a/cloud-hypervisor/tests/integration.rs b/cloud-hypervisor/tests/integration.rs index 7b085a487c..21f34487b4 100644 --- a/cloud-hypervisor/tests/integration.rs +++ b/cloud-hypervisor/tests/integration.rs @@ -1866,6 +1866,13 @@ mod common_parallel { _test_dmi_oem_strings(&guest); } + #[test] + #[cfg(target_arch = "x86_64")] + fn test_dmi_system_and_chassis() { + let guest = basic_regular_guest!(JAMMY_IMAGE_NAME); + _test_dmi_system_and_chassis(&guest); + } + #[test] fn test_virtio_fs() { _test_virtio_fs(&prepare_virtiofsd, false, false, None); diff --git a/cloud-hypervisor/tests/integration_cvm.rs b/cloud-hypervisor/tests/integration_cvm.rs index 0f4b17d771..1b6bd0c4bf 100644 --- a/cloud-hypervisor/tests/integration_cvm.rs +++ b/cloud-hypervisor/tests/integration_cvm.rs @@ -219,6 +219,12 @@ mod common_cvm { _test_dmi_oem_strings(&guest); } + #[test] + fn test_dmi_system_and_chassis() { + let guest = basic_cvm_guest!(JAMMY_IMAGE_NAME); + _test_dmi_system_and_chassis(&guest); + } + #[test] fn test_multiple_network_interfaces() { let guest = basic_cvm_guest!(JAMMY_IMAGE_NAME);