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 <sebastien.boeuf@intel.com>

Cargo.lock

@@ -602,6 +602,8 @@ dependencies = [
  "hypervisor",
  "libc",
  "log",
+ "serde",
+ "serde_derive",
  "thiserror",
  "versionize",
  "versionize_derive",

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"

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<E>(self, v: &str) -> Result<Self::Value, E>
+    where
+        E: serde::de::Error,
+    {
+        Ok(v.into())
+    }
+}
+
+impl<'de> serde::Deserialize<'de> for PciBdf {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        deserializer.deserialize_str(PciBdfVisitor)
+    }
+}
+
+impl serde::Serialize for PciBdf {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    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<Self, Self::Err> {
+        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()
+    }
+}

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<Mutex<dyn Migratable>>);
-        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);

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<String>,
     #[serde(skip)]
     pub migratable: Option<Arc<Mutex<dyn Migratable>>>,
-    pub pci_bdf: Option<u32>,
+    pub pci_bdf: Option<PciBdf>,
     #[serde(skip)]
     pub pci_device_handle: Option<PciDeviceHandle>,
 }
@@ -83,7 +84,7 @@ impl DeviceTree {
             .collect()
     }
 
-    pub fn remove_node_by_pci_bdf(&mut self, pci_bdf: u32) -> Option<DeviceNode> {
+    pub fn remove_node_by_pci_bdf(&mut self, pci_bdf: PciBdf) -> Option<DeviceNode> {
         let mut id = None;
         for (k, v) in self.0.iter() {
             if v.pci_bdf == Some(pci_bdf) {