From 42b5d4a2f7820469bd538ddd19d7f33eea6f68a8 Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Wed, 16 Feb 2022 09:55:26 +0100 Subject: [PATCH] pci, vmm: Update DeviceNode to store PciBdf instead of u32 By having the DeviceNode storing a PciBdf, we simplify the internal code as well as allow for custom Serialize/Deserialize implementation for the PciBdf structure. These custom implementations let us display the PCI s/b/d/f in a human readable format. Fixes #3711 Signed-off-by: Sebastien Boeuf --- Cargo.lock | 2 ++ pci/Cargo.toml | 2 ++ pci/src/lib.rs | 62 ++++++++++++++++++++++++++++++++++++++- vmm/src/device_manager.rs | 14 ++++----- vmm/src/device_tree.rs | 5 ++-- 5 files changed, 74 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d2ba6e46a..4d89367c8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -602,6 +602,8 @@ dependencies = [ "hypervisor", "libc", "log", + "serde", + "serde_derive", "thiserror", "versionize", "versionize_derive", diff --git a/pci/Cargo.toml b/pci/Cargo.toml index 658516612..df0c51735 100644 --- a/pci/Cargo.toml +++ b/pci/Cargo.toml @@ -18,6 +18,8 @@ vfio_user = { path = "../vfio_user" } vmm-sys-util = "0.9.0" libc = "0.2.118" log = "0.4.14" +serde = "1.0.136" +serde_derive = "1.0.136" thiserror = "1.0.30" versionize = "0.1.6" versionize_derive = "0.1.4" diff --git a/pci/src/lib.rs b/pci/src/lib.rs index c39c6fd30..45403ff24 100644 --- a/pci/src/lib.rs +++ b/pci/src/lib.rs @@ -27,7 +27,10 @@ pub use self::msi::{msi_num_enabled_vectors, MsiCap, MsiConfig}; pub use self::msix::{MsixCap, MsixConfig, MsixTableEntry, MSIX_TABLE_ENTRY_SIZE}; pub use self::vfio::{VfioPciDevice, VfioPciError}; pub use self::vfio_user::{VfioUserDmaMapping, VfioUserPciDevice, VfioUserPciDeviceError}; -use std::fmt::Display; +use serde::de::Visitor; +use std::fmt::{self, Display}; +use std::num::ParseIntError; +use std::str::FromStr; /// PCI has four interrupt pins A->D. #[derive(Copy, Clone)] @@ -52,6 +55,41 @@ pub const PCI_CONFIG_IO_PORT_SIZE: u64 = 0x8; #[derive(Clone, Copy, PartialEq, PartialOrd)] pub struct PciBdf(u32); +struct PciBdfVisitor; + +impl<'de> Visitor<'de> for PciBdfVisitor { + type Value = PciBdf; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("struct PciBdf") + } + + fn visit_str(self, v: &str) -> Result + where + E: serde::de::Error, + { + Ok(v.into()) + } +} + +impl<'de> serde::Deserialize<'de> for PciBdf { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + deserializer.deserialize_str(PciBdfVisitor) + } +} + +impl serde::Serialize for PciBdf { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + serializer.collect_str(&self.to_string()) + } +} + impl PciBdf { pub fn segment(&self) -> u16 { ((self.0 >> 16) & 0xffff) as u16 @@ -121,3 +159,25 @@ impl Display for PciBdf { ) } } + +impl FromStr for PciBdf { + type Err = ParseIntError; + + fn from_str(s: &str) -> Result { + let items: Vec<&str> = s.split('.').collect(); + assert_eq!(items.len(), 2); + let function = u8::from_str_radix(items[1], 16)?; + let items: Vec<&str> = items[0].split(':').collect(); + assert_eq!(items.len(), 3); + let segment = u16::from_str_radix(items[0], 16)?; + let bus = u8::from_str_radix(items[1], 16)?; + let device = u8::from_str_radix(items[2], 16)?; + Ok(PciBdf::new(segment, bus, device, function)) + } +} + +impl From<&str> for PciBdf { + fn from(bdf: &str) -> Self { + Self::from_str(bdf).unwrap() + } +} diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index 67280228b..ca29bb750 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -3082,7 +3082,7 @@ impl DeviceManager { }); } - node.pci_bdf = Some(pci_device_bdf.into()); + node.pci_bdf = Some(pci_device_bdf); node.pci_device_handle = Some(PciDeviceHandle::Vfio(vfio_pci_device)); self.device_tree @@ -3241,7 +3241,7 @@ impl DeviceManager { let mut node = device_node!(vfio_user_name); - node.pci_bdf = Some(pci_device_bdf.into()); + node.pci_bdf = Some(pci_device_bdf); node.pci_device_handle = Some(PciDeviceHandle::VfioUser(vfio_user_pci_device)); self.device_tree @@ -3288,8 +3288,7 @@ impl DeviceManager { info!("Restoring virtio-pci {} resources", id); let pci_device_bdf: PciBdf = node .pci_bdf - .ok_or(DeviceManagerError::MissingDeviceNodePciBdf)? - .into(); + .ok_or(DeviceManagerError::MissingDeviceNodePciBdf)?; let pci_segment_id = pci_device_bdf.segment(); self.pci_segments[pci_segment_id as usize] @@ -3397,7 +3396,7 @@ impl DeviceManager { }); } node.migratable = Some(Arc::clone(&virtio_pci_device) as Arc>); - node.pci_bdf = Some(pci_device_bdf.into()); + node.pci_bdf = Some(pci_device_bdf); node.pci_device_handle = Some(PciDeviceHandle::Virtio(virtio_pci_device)); self.device_tree.lock().unwrap().insert(id, node); @@ -3570,8 +3569,7 @@ impl DeviceManager { let pci_device_bdf: PciBdf = pci_device_node .pci_bdf - .ok_or(DeviceManagerError::MissingDeviceNodePciBdf)? - .into(); + .ok_or(DeviceManagerError::MissingDeviceNodePciBdf)?; let pci_segment_id = pci_device_bdf.segment(); let pci_device_handle = pci_device_node @@ -3625,7 +3623,7 @@ impl DeviceManager { // Remove the device from the device tree along with its children. let mut device_tree = self.device_tree.lock().unwrap(); let pci_device_node = device_tree - .remove_node_by_pci_bdf(pci_device_bdf.into()) + .remove_node_by_pci_bdf(pci_device_bdf) .ok_or(DeviceManagerError::MissingPciDevice)?; for child in pci_device_node.children.iter() { device_tree.remove(child); diff --git a/vmm/src/device_tree.rs b/vmm/src/device_tree.rs index 54cb65a31..3ab0f7e09 100644 --- a/vmm/src/device_tree.rs +++ b/vmm/src/device_tree.rs @@ -3,6 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::device_manager::PciDeviceHandle; +use pci::PciBdf; use std::collections::HashMap; use std::sync::{Arc, Mutex}; use vm_device::Resource; @@ -16,7 +17,7 @@ pub struct DeviceNode { pub children: Vec, #[serde(skip)] pub migratable: Option>>, - pub pci_bdf: Option, + pub pci_bdf: Option, #[serde(skip)] pub pci_device_handle: Option, } @@ -83,7 +84,7 @@ impl DeviceTree { .collect() } - pub fn remove_node_by_pci_bdf(&mut self, pci_bdf: u32) -> Option { + pub fn remove_node_by_pci_bdf(&mut self, pci_bdf: PciBdf) -> Option { let mut id = None; for (k, v) in self.0.iter() { if v.pci_bdf == Some(pci_bdf) {