From 0141635a5c36fbc043dcef8c87420ba40dbab5f6 Mon Sep 17 00:00:00 2001 From: Leander Kohler Date: Mon, 9 Feb 2026 15:28:03 +0100 Subject: [PATCH] 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 d54674873..f9f53e2df 100644 --- a/arch/src/x86_64/mod.rs +++ b/arch/src/x86_64/mod.rs @@ -1061,7 +1061,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 @@ -1069,8 +1069,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 6f1139888..87c8f00d9 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 69d02e07b..75fada61c 100644 --- a/vmm/src/vm.rs +++ b/vmm/src/vm.rs @@ -1879,10 +1879,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(