pci: fixes in PCI crate

Define thiserror::Error and displaydoc::Display for various error types
in the vended PCI crate. This way we can embed them in our error types
downstream. Also export a few types and struct fields that were private
and we will be needing them.

Signed-off-by: Babis Chalios <[email protected]>

Author: bchalios

Cargo.lock

@@ -13,6 +13,7 @@ default = []
 
 [dependencies]
 byteorder = "1.5.0"
+displaydoc = "0.2.5"
 libc = "0.2.172"
 log = "0.4.27"
 serde = { version = "1.0.219", features = ["derive"] }

src/pci/Cargo.toml

@@ -24,7 +24,7 @@ const DEVICE_ID_INTEL_VIRT_PCIE_HOST: u16 = 0x0d57;
 const NUM_DEVICE_IDS: usize = 32;
 
 /// Errors for device manager.
-#[derive(Debug)]
+#[derive(Debug, thiserror::Error, displaydoc::Display)]
 pub enum PciRootError {
     /// Could not allocate device address space for the device.
     AllocateDeviceAddrs(PciDeviceError),
@@ -103,7 +103,7 @@ impl PciDevice for PciRoot {
 pub struct PciBus {
     /// Devices attached to this bus.
     /// Device 0 is host bridge.
-    devices: HashMap<u32, Arc<Mutex<dyn PciDevice>>>,
+    pub devices: HashMap<u32, Arc<Mutex<dyn PciDevice>>>,
     device_reloc: Arc<dyn DeviceRelocation>,
     device_ids: Vec<bool>,
 }

src/pci/src/bus.rs

@@ -409,7 +409,7 @@ struct PciBar {
     r#type: Option<PciBarRegionType>,
 }
 
-#[derive(Serialize, Deserialize)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct PciConfigurationState {
     registers: Vec<u32>,
     writable_bits: Vec<u32>,
@@ -466,7 +466,7 @@ impl From<PciBarRegionType> for PciBarType {
     }
 }
 
-#[derive(Copy, Clone)]
+#[derive(Debug, Copy, Clone, Serialize, Deserialize)]
 pub enum PciBarPrefetchable {
     NotPrefetchable = 0,
     Prefetchable = 0x08,
@@ -481,7 +481,7 @@ impl From<PciBarPrefetchable> for bool {
     }
 }
 
-#[derive(Copy, Clone)]
+#[derive(Debug, Copy, Clone, Serialize, Deserialize)]
 pub struct PciBarConfiguration {
     addr: u64,
     size: u64,

src/pci/src/configuration.rs

@@ -6,7 +6,6 @@
 // SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause
 
 use std::any::Any;
-use std::fmt::{self, Display};
 use std::sync::{Arc, Barrier};
 use std::{io, result};
 
@@ -16,39 +15,21 @@ use vm_device::Resource;
 use crate::configuration::{self, PciBarRegionType};
 use crate::PciBarConfiguration;
 
-#[derive(Debug)]
+#[derive(Debug, thiserror::Error, displaydoc::Display)]
 pub enum Error {
-    /// Setup of the device capabilities failed.
+    /// Setup of the device capabilities failed: {0}.
     CapabilitiesSetup(configuration::Error),
-    /// Allocating space for an IO BAR failed.
+    /// Allocating space for an IO BAR failed, size={0}.
     IoAllocationFailed(u64),
-    /// Registering an IO BAR failed.
+    /// Registering an IO BAR at address {0} failed: {1}
     IoRegistrationFailed(u64, configuration::Error),
     /// Expected resource not found.
     MissingResource,
-    /// Invalid resource.
+    /// Invalid resource
     InvalidResource(Resource),
 }
 pub type Result<T> = std::result::Result<T, Error>;
 
-impl Display for Error {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        use self::Error::*;
-
-        match self {
-            CapabilitiesSetup(e) => write!(f, "failed to add capability {e}"),
-            IoAllocationFailed(size) => {
-                write!(f, "failed to allocate space for an IO BAR, size={size}")
-            }
-            IoRegistrationFailed(addr, e) => {
-                write!(f, "failed to register an IO BAR, addr={addr} err={e}")
-            }
-            MissingResource => write!(f, "failed to find expected resource"),
-            InvalidResource(r) => write!(f, "invalid resource {r:?}"),
-        }
-    }
-}
-
 #[derive(Clone, Copy)]
 pub struct BarReprogrammingParams {
     pub old_base: u64,

src/pci/src/device.rs

@@ -24,15 +24,18 @@ use serde::de::Visitor;
 pub use self::bus::{PciBus, PciConfigIo, PciConfigMmio, PciRoot, PciRootError};
 pub use self::configuration::{
     PciBarConfiguration, PciBarPrefetchable, PciBarRegionType, PciCapability, PciCapabilityId,
-    PciClassCode, PciConfiguration, PciExpressCapabilityId, PciHeaderType, PciMassStorageSubclass,
-    PciNetworkControllerSubclass, PciProgrammingInterface, PciSerialBusSubClass, PciSubclass,
-    PCI_CONFIGURATION_ID,
+    PciClassCode, PciConfiguration, PciConfigurationState, PciExpressCapabilityId, PciHeaderType,
+    PciMassStorageSubclass, PciNetworkControllerSubclass, PciProgrammingInterface,
+    PciSerialBusSubClass, PciSubclass, PCI_CONFIGURATION_ID,
 };
 pub use self::device::{
     BarReprogrammingParams, DeviceRelocation, Error as PciDeviceError, PciDevice,
 };
 pub use self::msi::{msi_num_enabled_vectors, MsiCap, MsiConfig};
-pub use self::msix::{MsixCap, MsixConfig, MsixTableEntry, MSIX_CONFIG_ID, MSIX_TABLE_ENTRY_SIZE};
+pub use self::msix::{
+    Error as MsixError, MsixCap, MsixConfig, MsixConfigState, MsixTableEntry, MSIX_CONFIG_ID,
+    MSIX_TABLE_ENTRY_SIZE,
+};
 
 /// PCI has four interrupt pins A->D.
 #[derive(Copy, Clone)]

src/pci/src/lib.rs

@@ -26,7 +26,7 @@ const MSIX_ENABLE_MASK: u16 = (1 << MSIX_ENABLE_BIT) as u16;
 pub const MSIX_TABLE_ENTRY_SIZE: usize = 16;
 pub const MSIX_CONFIG_ID: &str = "msix_config";
 
-#[derive(Debug)]
+#[derive(Debug, thiserror::Error, displaydoc::Display)]
 pub enum Error {
     /// Failed enabling the interrupt route.
     EnableInterruptRoute(io::Error),
@@ -59,7 +59,7 @@ impl Default for MsixTableEntry {
     }
 }
 
-#[derive(Serialize, Deserialize)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct MsixConfigState {
     table_entries: Vec<MsixTableEntry>,
     pba_entries: Vec<u64>,
@@ -71,11 +71,23 @@ pub struct MsixConfig {
     pub table_entries: Vec<MsixTableEntry>,
     pub pba_entries: Vec<u64>,
     pub devid: u32,
-    interrupt_source_group: Arc<dyn InterruptSourceGroup>,
+    pub interrupt_source_group: Arc<dyn InterruptSourceGroup>,
     masked: bool,
     enabled: bool,
 }
 
+impl std::fmt::Debug for MsixConfig {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("MsixConfig")
+            .field("table_entries", &self.table_entries)
+            .field("pba_entries", &self.pba_entries)
+            .field("devid", &self.devid)
+            .field("masked", &self.masked)
+            .field("enabled", &self.enabled)
+            .finish()
+    }
+}
+
 impl MsixConfig {
     pub fn new(
         msix_vectors: u16,