diff --git a/openvmm/hvlite_core/src/worker/dispatch.rs b/openvmm/hvlite_core/src/worker/dispatch.rs index b64a8e4ca9..ae1ae2e4a4 100644 --- a/openvmm/hvlite_core/src/worker/dispatch.rs +++ b/openvmm/hvlite_core/src/worker/dispatch.rs @@ -1786,6 +1786,7 @@ impl InitializedVm { .into_iter() .map(|rp_cfg| GenericPcieRootPortDefinition { name: rp_cfg.name.into(), + hotplug: rp_cfg.hotplug, }) .collect(); @@ -1828,7 +1829,8 @@ impl InitializedVm { .add(|_services| { let definition = pcie::switch::GenericPcieSwitchDefinition { name: switch.name.clone().into(), - downstream_port_count: switch.num_downstream_ports as usize, + downstream_port_count: switch.num_downstream_ports, + hotplug: switch.hotplug, }; GenericPcieSwitch::new(definition) })?; diff --git a/openvmm/hvlite_defs/src/config.rs b/openvmm/hvlite_defs/src/config.rs index b19a139df4..2620779f59 100644 --- a/openvmm/hvlite_defs/src/config.rs +++ b/openvmm/hvlite_defs/src/config.rs @@ -186,6 +186,7 @@ pub struct PcieRootComplexConfig { #[derive(Debug, MeshPayload)] pub struct PcieRootPortConfig { pub name: String, + pub hotplug: bool, } #[derive(Debug, MeshPayload)] @@ -193,6 +194,7 @@ pub struct PcieSwitchConfig { pub name: String, pub num_downstream_ports: u8, pub parent_port: String, + pub hotplug: bool, } #[derive(Debug, MeshPayload)] diff --git a/openvmm/openvmm_entry/src/cli_args.rs b/openvmm/openvmm_entry/src/cli_args.rs index babd537674..8bd2a939a6 100644 --- a/openvmm/openvmm_entry/src/cli_args.rs +++ b/openvmm/openvmm_entry/src/cli_args.rs @@ -571,11 +571,15 @@ options: /// Attach a PCI Express root complex to the VM #[clap(long_help = r#" -e.g: --pcie-root-complex rc0,segment=0,start_bus=0,end_bus=255,low_mmio=4M,high_mmio=1G +Attach root complexes to the VM. -syntax: [,opt=arg,...] +Examples: + # Attach root complex rc0 on segment 0 with bus and MMIO ranges + --pcie-root-complex rc0,segment=0,start_bus=0,end_bus=255,low_mmio=4M,high_mmio=1G -options: +Syntax: [,opt=arg,...] + +Options: `segment=` configures the PCI Express segment, default 0 `start_bus=` lowest valid bus number, default 0 `end_bus=` highest valid bus number, default 255 @@ -587,22 +591,32 @@ options: /// Attach a PCI Express root port to the VM #[clap(long_help = r#" -e.g: --pcie-root-port rc0:rc0rp0 +Attach root ports to root complexes. + +Examples: + # Attach root port rc0rp0 to root complex rc0 + --pcie-root-port rc0:rc0rp0 + + # Attach root port rc0rp1 to root complex rc0 with hotplug support + --pcie-root-port rc0:rc0rp1,hotplug -syntax: : +Syntax: :[,hotplug] + +Options: + `hotplug` enable hotplug support for this root port "#)] #[clap(long, conflicts_with("pcat"))] pub pcie_root_port: Vec, /// Attach a PCI Express switch to the VM #[clap(long_help = r#" -Attach switches to root ports or other switches to create PCIe hierarchies. +Attach switches to root ports or downstream switch ports to create PCIe hierarchies. Examples: - # Connect switch0 directly to root port rp0 + # Connect switch0 (with 4 downstream switch ports) directly to root port rp0 --pcie-switch rp0:switch0,num_downstream_ports=4 - # Connect switch1 to downstream port 0 of switch0 + # Connect switch1 (with 2 downstream switch ports) to downstream port 0 of switch0 --pcie-switch switch0-downstream-0:switch1,num_downstream_ports=2 # Create a 3-level hierarchy: rp0 -> switch0 -> switch1 -> switch2 @@ -610,14 +624,18 @@ Examples: --pcie-switch switch0-downstream-0:switch1 --pcie-switch switch1-downstream-1:switch2 -syntax: :[,opt=arg,...] + # Enable hotplug on all downstream switch ports of switch0 + --pcie-switch rp0:switch0,hotplug -port_name can be: - - Root port name (e.g., "rp0") to connect directly to a root port - - Downstream port name (e.g., "switch0-downstream-1") to connect to another switch +Syntax: :[,opt,opt=arg,...] -options: - `num_downstream_ports=` number of downstream ports, default 4 + port_name can be: + - Root port name (e.g., "rp0") to connect directly to a root port + - Downstream port name (e.g., "switch0-downstream-1") to connect to another switch + +Options: + `hotplug` enable hotplug support for all downstream switch ports + `num_downstream_ports=` number of downstream ports, default 4 "#)] #[clap(long, conflicts_with("pcat"))] pub pcie_switch: Vec, @@ -1534,6 +1552,7 @@ impl FromStr for PcieRootComplexCli { pub struct PcieRootPortCli { pub root_complex_name: String, pub name: String, + pub hotplug: bool, } impl FromStr for PcieRootPortCli { @@ -1554,13 +1573,20 @@ impl FromStr for PcieRootPortCli { anyhow::bail!("unexpected token: '{extra}'") } - if let Some(extra) = opts.next() { - anyhow::bail!("unexpected token: '{extra}'") + let mut hotplug = false; + + // Parse optional flags + for opt in opts { + match opt { + "hotplug" => hotplug = true, + _ => anyhow::bail!("unexpected option: '{opt}'"), + } } Ok(PcieRootPortCli { root_complex_name: rc_name.to_string(), name: rp_name.to_string(), + hotplug, }) } } @@ -1570,6 +1596,7 @@ pub struct GenericPcieSwitchCli { pub port_name: String, pub name: String, pub num_downstream_ports: u8, + pub hotplug: bool, } impl FromStr for GenericPcieSwitchCli { @@ -1591,20 +1618,26 @@ impl FromStr for GenericPcieSwitchCli { } let mut num_downstream_ports = 4u8; // Default value + let mut hotplug = false; for opt in opts { let mut kv = opt.split('='); let key = kv.next().context("expected option name")?; - let value = kv.next().context("expected option value")?; - - if let Some(extra) = kv.next() { - anyhow::bail!("unexpected token: '{extra}'") - } match key { "num_downstream_ports" => { + let value = kv.next().context("expected option value")?; + if let Some(extra) = kv.next() { + anyhow::bail!("unexpected token: '{extra}'") + } num_downstream_ports = value.parse().context("invalid num_downstream_ports")?; } + "hotplug" => { + if kv.next().is_some() { + anyhow::bail!("hotplug option does not take a value") + } + hotplug = true; + } _ => anyhow::bail!("unknown option: '{key}'"), } } @@ -1613,6 +1646,7 @@ impl FromStr for GenericPcieSwitchCli { port_name: port_name.to_string(), name: switch_name.to_string(), num_downstream_ports, + hotplug, }) } } @@ -2237,7 +2271,8 @@ mod tests { PcieRootPortCli::from_str("rc0:rc0rp0").unwrap(), PcieRootPortCli { root_complex_name: "rc0".to_string(), - name: "rc0rp0".to_string() + name: "rc0rp0".to_string(), + hotplug: false, } ); @@ -2245,7 +2280,18 @@ mod tests { PcieRootPortCli::from_str("my_rc:port2").unwrap(), PcieRootPortCli { root_complex_name: "my_rc".to_string(), - name: "port2".to_string() + name: "port2".to_string(), + hotplug: false, + } + ); + + // Test with hotplug flag + assert_eq!( + PcieRootPortCli::from_str("my_rc:port2,hotplug").unwrap(), + PcieRootPortCli { + root_complex_name: "my_rc".to_string(), + name: "port2".to_string(), + hotplug: true, } ); @@ -2254,7 +2300,7 @@ mod tests { assert!(PcieRootPortCli::from_str("rp0").is_err()); assert!(PcieRootPortCli::from_str("rp0,opt").is_err()); assert!(PcieRootPortCli::from_str("rc0:rp0:rp3").is_err()); - assert!(PcieRootPortCli::from_str("rc0:rp0,rp3").is_err()); + assert!(PcieRootPortCli::from_str("rc0:rp0,invalid_option").is_err()); } #[test] @@ -2265,6 +2311,7 @@ mod tests { port_name: "rp0".to_string(), name: "switch0".to_string(), num_downstream_ports: 4, + hotplug: false, } ); @@ -2274,6 +2321,7 @@ mod tests { port_name: "port1".to_string(), name: "my_switch".to_string(), num_downstream_ports: 4, + hotplug: false, } ); @@ -2283,6 +2331,7 @@ mod tests { port_name: "rp2".to_string(), name: "sw".to_string(), num_downstream_ports: 8, + hotplug: false, } ); @@ -2293,6 +2342,29 @@ mod tests { port_name: "switch0-downstream-1".to_string(), name: "child_switch".to_string(), num_downstream_ports: 4, + hotplug: false, + } + ); + + // Test hotplug flag + assert_eq!( + GenericPcieSwitchCli::from_str("rp0:switch0,hotplug").unwrap(), + GenericPcieSwitchCli { + port_name: "rp0".to_string(), + name: "switch0".to_string(), + num_downstream_ports: 4, + hotplug: true, + } + ); + + // Test hotplug with num_downstream_ports + assert_eq!( + GenericPcieSwitchCli::from_str("rp0:switch0,num_downstream_ports=8,hotplug").unwrap(), + GenericPcieSwitchCli { + port_name: "rp0".to_string(), + name: "switch0".to_string(), + num_downstream_ports: 8, + hotplug: true, } ); @@ -2303,5 +2375,6 @@ mod tests { assert!(GenericPcieSwitchCli::from_str("rp0:switch0,invalid_opt=value").is_err()); assert!(GenericPcieSwitchCli::from_str("rp0:switch0,num_downstream_ports=bad").is_err()); assert!(GenericPcieSwitchCli::from_str("rp0:switch0,num_downstream_ports=").is_err()); + assert!(GenericPcieSwitchCli::from_str("rp0:switch0,invalid_flag").is_err()); } } diff --git a/openvmm/openvmm_entry/src/lib.rs b/openvmm/openvmm_entry/src/lib.rs index 65d5571275..e86411387d 100644 --- a/openvmm/openvmm_entry/src/lib.rs +++ b/openvmm/openvmm_entry/src/lib.rs @@ -221,6 +221,7 @@ fn build_switch_list(all_switches: &[cli_args::GenericPcieSwitchCli]) -> Vec &str; + /// Returns the PCI capability ID for this capability + fn capability_id(&self) -> CapabilityId; + /// Length of the capability structure fn len(&self) -> usize; @@ -28,4 +33,16 @@ pub trait PciCapability: Send + Sync + Inspect + ProtobufSaveRestore { /// Reset the capability fn reset(&mut self); + + // Specific downcast methods for known capability types + + /// Downcast to PCI Express capability + fn as_pci_express(&self) -> Option<&pci_express::PciExpressCapability> { + None + } + + /// Downcast to PCI Express capability (mutable) + fn as_pci_express_mut(&mut self) -> Option<&mut pci_express::PciExpressCapability> { + None + } } diff --git a/vm/devices/pci/pci_core/src/capabilities/msi_cap.rs b/vm/devices/pci/pci_core/src/capabilities/msi_cap.rs new file mode 100644 index 0000000000..c9cccd78d5 --- /dev/null +++ b/vm/devices/pci/pci_core/src/capabilities/msi_cap.rs @@ -0,0 +1,426 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! MSI Capability. + +use super::PciCapability; +use crate::msi::MsiInterrupt; +use crate::msi::RegisterMsi; +use crate::spec::caps::CapabilityId; +use crate::spec::caps::msi::MsiCapabilityHeader; +use inspect::Inspect; +use inspect::InspectMut; +use parking_lot::Mutex; +use std::fmt::Debug; +use std::sync::Arc; +use vmcore::interrupt::Interrupt; + +/// MSI capability implementation for PCI configuration space. +#[derive(Debug, Inspect)] +pub struct MsiCapability { + #[inspect(with = "|x| inspect::adhoc(|req| x.lock().inspect_mut(req))")] + state: Arc>, + addr_64bit: bool, + per_vector_masking: bool, +} + +#[derive(Debug, InspectMut)] +struct MsiCapabilityState { + enabled: bool, + multiple_message_enable: u8, // 2^(MME) messages allocated + multiple_message_capable: u8, // 2^(MMC) maximum messages requestable + address: u64, + data: u16, + mask_bits: u32, + pending_bits: u32, + interrupt: Option, +} + +impl MsiCapabilityState { + fn new(multiple_message_capable: u8, _addr_64bit: bool, per_vector_masking: bool) -> Self { + Self { + enabled: false, + multiple_message_enable: 0, + multiple_message_capable, + address: 0, + data: 0, + mask_bits: if per_vector_masking { 0xFFFFFFFF } else { 0 }, + pending_bits: 0, + interrupt: None, + } + } + + fn control_register(&self, addr_64bit: bool, per_vector_masking: bool) -> u32 { + let mut control = 0u32; + control |= (self.multiple_message_capable as u32) << 1; // MMC field (bits 1-3) + control |= (self.multiple_message_enable as u32) << 4; // MME field (bits 4-6) + if addr_64bit { + control |= 1 << 7; // 64-bit Address Capable (bit 7) + } + if per_vector_masking { + control |= 1 << 8; // Per-vector Masking Capable (bit 8) + } + if self.enabled { + control |= 1 << 0; // MSI Enable (bit 0) + } + control + } +} + +impl MsiCapability { + /// Create a new MSI capability. + /// + /// # Arguments + /// * `multiple_message_capable` - log2 of maximum number of messages (0-5) + /// * `addr_64bit` - Whether 64-bit addressing is supported + /// * `per_vector_masking` - Whether per-vector masking is supported + /// * `register_msi` - MSI registration interface + pub fn new( + multiple_message_capable: u8, + addr_64bit: bool, + per_vector_masking: bool, + register_msi: &mut dyn RegisterMsi, + ) -> Self { + assert!(multiple_message_capable <= 5, "MMC must be 0-5"); + + let interrupt = register_msi.new_msi(); + let state = MsiCapabilityState { + interrupt: Some(interrupt), + ..MsiCapabilityState::new(multiple_message_capable, addr_64bit, per_vector_masking) + }; + + Self { + state: Arc::new(Mutex::new(state)), + addr_64bit, + per_vector_masking, + } + } + + /// Get the interrupt object for signaling MSI. + pub fn interrupt(&self) -> Option { + self.state.lock().interrupt.as_mut().map(|i| i.interrupt()) + } + + fn len_bytes(&self) -> usize { + let mut len = 8; // Base: ID + Next + Control + Message Address Low + if self.addr_64bit { + len += 4; // Message Address High + } + len += 2; // Message Data (16-bit, but aligned to 4-byte boundary) + if self.per_vector_masking { + len += 8; // Mask Bits + Pending Bits + } + // Round up to next 4-byte boundary + (len + 3) & !3 + } +} + +impl PciCapability for MsiCapability { + fn label(&self) -> &str { + "msi" + } + + fn capability_id(&self) -> CapabilityId { + CapabilityId::MSI + } + + fn len(&self) -> usize { + self.len_bytes() + } + + fn read_u32(&self, offset: u16) -> u32 { + let state = self.state.lock(); + + match MsiCapabilityHeader(offset) { + MsiCapabilityHeader::CONTROL_CAPS => { + let control_reg = state.control_register(self.addr_64bit, self.per_vector_masking); + CapabilityId::MSI.0 as u32 | (control_reg << 16) + } + MsiCapabilityHeader::MSG_ADDR_LO => state.address as u32, + MsiCapabilityHeader::MSG_ADDR_HI if self.addr_64bit => (state.address >> 32) as u32, + MsiCapabilityHeader::MSG_DATA_32 if !self.addr_64bit => state.data as u32, + MsiCapabilityHeader::MSG_DATA_64 if self.addr_64bit => state.data as u32, + MsiCapabilityHeader::MASK_BITS if self.addr_64bit && self.per_vector_masking => { + state.mask_bits + } + MsiCapabilityHeader::PENDING_BITS if self.addr_64bit && self.per_vector_masking => { + state.pending_bits + } + _ => { + tracelimit::warn_ratelimited!("Unexpected MSI read offset {:#x}", offset); + 0 + } + } + } + + fn write_u32(&mut self, offset: u16, val: u32) { + let mut state = self.state.lock(); + + match MsiCapabilityHeader(offset) { + MsiCapabilityHeader::CONTROL_CAPS => { + let control_val = (val >> 16) & 0xFFFF; + let old_enabled = state.enabled; + let new_enabled = control_val & 1 != 0; + let mme = ((control_val >> 4) & 0x7) as u8; + + // Update MME (Multiple Message Enable) - limited by MMC + state.multiple_message_enable = mme.min(state.multiple_message_capable); + state.enabled = new_enabled; + + // Handle enable/disable state changes + let address = state.address; + let data = state.data as u32; + if let Some(ref mut interrupt) = state.interrupt { + if new_enabled && !old_enabled { + // Enable MSI + interrupt.enable(address, data, false); + } else if !new_enabled && old_enabled { + // Disable MSI + interrupt.disable(); + } + } + } + MsiCapabilityHeader::MSG_ADDR_LO => { + state.address = (state.address & 0xFFFFFFFF00000000) | (val as u64); + + // Update interrupt if enabled + if state.enabled { + let address = state.address; + let data = state.data as u32; + if let Some(ref mut interrupt) = state.interrupt { + interrupt.enable(address, data, false); + } + } + } + MsiCapabilityHeader::MSG_ADDR_HI if self.addr_64bit => { + state.address = (state.address & 0xFFFFFFFF) | ((val as u64) << 32); + + // Update interrupt if enabled + if state.enabled { + let address = state.address; + let data = state.data as u32; + if let Some(ref mut interrupt) = state.interrupt { + interrupt.enable(address, data, false); + } + } + } + MsiCapabilityHeader::MSG_DATA_32 if !self.addr_64bit => { + state.data = val as u16; + + // Update interrupt if enabled + if state.enabled { + let address = state.address; + let data = state.data as u32; + if let Some(ref mut interrupt) = state.interrupt { + interrupt.enable(address, data, false); + } + } + } + MsiCapabilityHeader::MSG_DATA_64 if self.addr_64bit => { + state.data = val as u16; + + // Update interrupt if enabled + if state.enabled { + let address = state.address; + let data = state.data as u32; + if let Some(ref mut interrupt) = state.interrupt { + interrupt.enable(address, data, false); + } + } + } + MsiCapabilityHeader::MASK_BITS if self.addr_64bit && self.per_vector_masking => { + state.mask_bits = val; + } + MsiCapabilityHeader::PENDING_BITS if self.addr_64bit && self.per_vector_masking => { + // Pending bits are typically read-only, but some implementations may allow clearing + tracelimit::warn_ratelimited!( + "Write to MSI pending bits register (typically read-only)" + ); + } + _ => { + tracelimit::warn_ratelimited!("Unexpected MSI write offset {:#x}", offset); + } + } + } + + fn reset(&mut self) { + let mut state = self.state.lock(); + + // Disable MSI + if state.enabled { + if let Some(ref mut interrupt) = state.interrupt { + interrupt.disable(); + } + } + + // Reset to default values + state.enabled = false; + state.multiple_message_enable = 0; + state.address = 0; + state.data = 0; + if self.per_vector_masking { + state.mask_bits = 0; + state.pending_bits = 0; + } + } +} + +mod save_restore { + use super::*; + use thiserror::Error; + use vmcore::save_restore::RestoreError; + use vmcore::save_restore::SaveError; + use vmcore::save_restore::SaveRestore; + + mod state { + use mesh::payload::Protobuf; + use vmcore::save_restore::SavedStateRoot; + + #[derive(Debug, Protobuf, SavedStateRoot)] + #[mesh(package = "pci.caps.msi")] + pub struct SavedState { + #[mesh(1)] + pub enabled: bool, + #[mesh(2)] + pub multiple_message_enable: u8, + #[mesh(3)] + pub address: u64, + #[mesh(4)] + pub data: u16, + #[mesh(5)] + pub mask_bits: u32, + #[mesh(6)] + pub pending_bits: u32, + } + } + + #[derive(Debug, Error)] + enum MsiRestoreError { + #[error("invalid multiple message enable value: {0}")] + InvalidMultipleMessageEnable(u8), + } + + impl SaveRestore for MsiCapability { + type SavedState = state::SavedState; + + fn save(&mut self) -> Result { + let state = self.state.lock(); + Ok(state::SavedState { + enabled: state.enabled, + multiple_message_enable: state.multiple_message_enable, + address: state.address, + data: state.data, + mask_bits: state.mask_bits, + pending_bits: state.pending_bits, + }) + } + + fn restore(&mut self, saved_state: Self::SavedState) -> Result<(), RestoreError> { + let state::SavedState { + enabled, + multiple_message_enable, + address, + data, + mask_bits, + pending_bits, + } = saved_state; + + if multiple_message_enable > 5 { + return Err(RestoreError::InvalidSavedState( + MsiRestoreError::InvalidMultipleMessageEnable(multiple_message_enable).into(), + )); + } + + let mut state = self.state.lock(); + + // Disable current interrupt if needed + if state.enabled { + if let Some(ref mut interrupt) = state.interrupt { + interrupt.disable(); + } + } + + // Restore state + state.enabled = enabled; + state.multiple_message_enable = + multiple_message_enable.min(state.multiple_message_capable); + state.address = address; + state.data = data; + state.mask_bits = mask_bits; + state.pending_bits = pending_bits; + + // Re-enable interrupt if needed + if state.enabled { + let address = state.address; + let data = state.data as u32; + if let Some(ref mut interrupt) = state.interrupt { + interrupt.enable(address, data, false); + } + } + + Ok(()) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::msi::MsiInterruptSet; + use crate::test_helpers::TestPciInterruptController; + + #[test] + fn msi_check() { + let mut set = MsiInterruptSet::new(); + let mut cap = MsiCapability::new(2, true, false, &mut set); // 4 messages max, 64-bit, no masking + let msi_controller = TestPciInterruptController::new(); + set.connect(&msi_controller); + + // Check initial capabilities register + // Capability ID (0x05) + MMC=2 (4 messages) + 64-bit capable + assert_eq!(cap.read_u32(0), 0x00840005); // 0x05 (ID) | (0x84 << 16) where 0x84 = MMC=2(<<1) + 64bit(<<7) + + // Check initial address registers + assert_eq!(cap.read_u32(4), 0); // Address low + assert_eq!(cap.read_u32(8), 0); // Address high + assert_eq!(cap.read_u32(12), 0); // Data + + // Write address and data + cap.write_u32(4, 0x12345678); + cap.write_u32(8, 0x9abcdef0); + cap.write_u32(12, 0x1234); + + assert_eq!(cap.read_u32(4), 0x12345678); + assert_eq!(cap.read_u32(8), 0x9abcdef0); + assert_eq!(cap.read_u32(12), 0x1234); + + // Enable MSI with 2 messages (MME=1) + cap.write_u32(0, 0x00110005); // Enable + MME=1 (bits 0 and 4-6) + assert_eq!(cap.read_u32(0), 0x00950005); // Should show enabled with all capability bits + + // Test reset + cap.reset(); + assert_eq!(cap.read_u32(0), 0x00840005); // Back to disabled + assert_eq!(cap.read_u32(4), 0); + assert_eq!(cap.read_u32(8), 0); + assert_eq!(cap.read_u32(12), 0); + } + + #[test] + fn msi_32bit_check() { + let mut set = MsiInterruptSet::new(); + let mut cap = MsiCapability::new(1, false, false, &mut set); // 2 messages max, 32-bit, no masking + let msi_controller = TestPciInterruptController::new(); + set.connect(&msi_controller); + + // Check initial capabilities register (no 64-bit bit set) + assert_eq!(cap.read_u32(0), 0x00020005); // MMC=1 (2 messages) + Capability ID + + // For 32-bit, data is at offset 8, not 12 + cap.write_u32(4, 0x12345678); // Address + cap.write_u32(8, 0x1234); // Data + + assert_eq!(cap.read_u32(4), 0x12345678); + assert_eq!(cap.read_u32(8), 0x1234); + } +} diff --git a/vm/devices/pci/pci_core/src/capabilities/msix.rs b/vm/devices/pci/pci_core/src/capabilities/msix.rs index 6c794cd9d0..cbd70ac7ae 100644 --- a/vm/devices/pci/pci_core/src/capabilities/msix.rs +++ b/vm/devices/pci/pci_core/src/capabilities/msix.rs @@ -49,6 +49,10 @@ impl PciCapability for MsixCapability { "msi-x" } + fn capability_id(&self) -> CapabilityId { + CapabilityId::MSIX + } + fn len(&self) -> usize { 12 } diff --git a/vm/devices/pci/pci_core/src/capabilities/pci_express.rs b/vm/devices/pci/pci_core/src/capabilities/pci_express.rs index 29e3eb4d53..3d35950b5f 100644 --- a/vm/devices/pci/pci_core/src/capabilities/pci_express.rs +++ b/vm/devices/pci/pci_core/src/capabilities/pci_express.rs @@ -6,7 +6,9 @@ use super::PciCapability; use crate::spec::caps::CapabilityId; use crate::spec::caps::pci_express; -use crate::spec::caps::pci_express::PciExpressCapabilityHeader; +use crate::spec::caps::pci_express::{ + LinkSpeed, LinkWidth, PciExpressCapabilityHeader, SupportedLinkSpeedsVector, +}; use inspect::Inspect; use parking_lot::Mutex; use std::sync::Arc; @@ -44,14 +46,17 @@ impl PciExpressState { device_control: pci_express::DeviceControl::new(), device_status: pci_express::DeviceStatus::new(), link_control: pci_express::LinkControl::new(), - link_status: pci_express::LinkStatus::new(), + link_status: pci_express::LinkStatus::new() + .with_current_link_speed(LinkSpeed::Speed32_0GtS.into_bits() as u16) + .with_negotiated_link_width(LinkWidth::X16.into_bits() as u16), slot_control: pci_express::SlotControl::new(), slot_status: pci_express::SlotStatus::new(), root_control: pci_express::RootControl::new(), root_status: pci_express::RootStatus::new(), device_control_2: pci_express::DeviceControl2::new(), device_status_2: pci_express::DeviceStatus2::new(), - link_control_2: pci_express::LinkControl2::new(), + link_control_2: pci_express::LinkControl2::new() + .with_target_link_speed(LinkSpeed::Speed32_0GtS.into_bits() as u16), link_status_2: pci_express::LinkStatus2::new(), slot_control_2: pci_express::SlotControl2::new(), slot_status_2: pci_express::SlotStatus2::new(), @@ -88,18 +93,23 @@ impl PciExpressCapability { .with_device_port_type(typ), device_capabilities: pci_express::DeviceCapabilities::new() .with_function_level_reset(flr_handler.is_some()), - link_capabilities: pci_express::LinkCapabilities::new(), + link_capabilities: pci_express::LinkCapabilities::new() + .with_max_link_speed(LinkSpeed::Speed32_0GtS.into_bits()) // PCIe 32.0 GT/s speed + .with_max_link_width(LinkWidth::X16.into_bits()), // x16 link width slot_capabilities: pci_express::SlotCapabilities::new(), root_capabilities: pci_express::RootCapabilities::new(), device_capabilities_2: pci_express::DeviceCapabilities2::new(), - link_capabilities_2: pci_express::LinkCapabilities2::new(), + link_capabilities_2: pci_express::LinkCapabilities2::new() + .with_supported_link_speeds_vector(SupportedLinkSpeedsVector::UpToGen5.into_bits()), // Support speeds up to PCIe Gen 5 (32.0 GT/s) slot_capabilities_2: pci_express::SlotCapabilities2::new(), state: Arc::new(Mutex::new(PciExpressState::new())), flr_handler, } } - fn handle_device_control_write(&mut self, new_control: pci_express::DeviceControl) { + fn handle_device_control_status_write(&mut self, val: u32) { + // Device Control (2 bytes) + Device Status (2 bytes) + let new_control = pci_express::DeviceControl::from_bits(val as u16); let mut state = self.state.lock(); // Check if FLR was initiated @@ -117,6 +127,210 @@ impl PciExpressCapability { // Update the control register but clear the FLR bit as it's self-clearing state.device_control = new_control.with_initiate_function_level_reset(false); + + // Handle Device Status - most bits are write-1-to-clear + let new_status = pci_express::DeviceStatus::from_bits((val >> 16) as u16); + let mut current_status = state.device_status; + + // Clear bits that were written as 1 (write-1-to-clear semantics) + if new_status.correctable_error_detected() { + current_status.set_correctable_error_detected(false); + } + if new_status.non_fatal_error_detected() { + current_status.set_non_fatal_error_detected(false); + } + if new_status.fatal_error_detected() { + current_status.set_fatal_error_detected(false); + } + if new_status.unsupported_request_detected() { + current_status.set_unsupported_request_detected(false); + } + + state.device_status = current_status; + } + + fn handle_slot_control_status_write(&mut self, val: u32) { + // Slot Control (2 bytes) + Slot Status (2 bytes) + let new_slot_control = pci_express::SlotControl::from_bits(val as u16); + let mut state = self.state.lock(); + + // Mask slot control bits based on slot capabilities + // Only allow writes to bits that correspond to capabilities that are present + let mut masked_control = new_slot_control; + + // If attention button is not present, attention button enable should be read-only (hardwired to 0) + if !self.slot_capabilities.attention_button_present() { + masked_control.set_attention_button_pressed_enable(false); + } + + // If power controller is not present, power controller control should be read-only (hardwired to 0) + if !self.slot_capabilities.power_controller_present() { + masked_control.set_power_controller_control(false); + } + + // If MRL sensor is not present, MRL sensor changed enable should be read-only (hardwired to 0) + if !self.slot_capabilities.mrl_sensor_present() { + masked_control.set_mrl_sensor_changed_enable(false); + } + + // If attention indicator is not present, attention indicator control should be read-only (hardwired to 00b) + if !self.slot_capabilities.attention_indicator_present() { + masked_control.set_attention_indicator_control(0); + } + + // If power indicator is not present, power indicator control should be read-only (hardwired to 00b) + if !self.slot_capabilities.power_indicator_present() { + masked_control.set_power_indicator_control(0); + } + + // If hotplug is not capable, hotplug interrupt enable should be read-only (hardwired to 0) + if !self.slot_capabilities.hot_plug_capable() { + masked_control.set_hot_plug_interrupt_enable(false); + } + + // If electromechanical interlock is not present, interlock control should be read-only (hardwired to 0) + if !self.slot_capabilities.electromechanical_interlock_present() { + masked_control.set_electromechanical_interlock_control(false); + } + + // If no command completed support, command completed interrupt enable should be read-only (hardwired to 0) + if self.slot_capabilities.no_command_completed_support() { + masked_control.set_command_completed_interrupt_enable(false); + } + + state.slot_control = masked_control; + + // Slot Status upper 16 bits - handle RW1C and RO bits properly + let new_slot_status = pci_express::SlotStatus::from_bits((val >> 16) as u16); + let mut current_slot_status = state.slot_status; + + // RW1C bits: writing 1 clears the bit, writing 0 leaves it unchanged + // Clear bits where a 1 was written (RW1C behavior) + if new_slot_status.attention_button_pressed() { + current_slot_status.set_attention_button_pressed(false); + } + if new_slot_status.power_fault_detected() { + current_slot_status.set_power_fault_detected(false); + } + if new_slot_status.mrl_sensor_changed() { + current_slot_status.set_mrl_sensor_changed(false); + } + if new_slot_status.presence_detect_changed() { + current_slot_status.set_presence_detect_changed(false); + } + if new_slot_status.command_completed() { + current_slot_status.set_command_completed(false); + } + if new_slot_status.data_link_layer_state_changed() { + current_slot_status.set_data_link_layer_state_changed(false); + } + + // RO bits (mrl_sensor_state, presence_detect_state, electromechanical_interlock_status) + // are not modified - they remain as they were + + state.slot_status = current_slot_status; + } + + fn handle_link_control_status_write(&mut self, val: u32) { + // Link Control (2 bytes) + Link Status (2 bytes) + let new_link_control = pci_express::LinkControl::from_bits(val as u16); + let mut state = self.state.lock(); + + // Apply the new link control but ensure retrain_link always reads as 0 + let mut masked_control = new_link_control; + masked_control.set_retrain_link(false); // retrain_link always reads as 0 + + state.link_control = masked_control; + // Link Status upper 16 bits - read-only, ignore any writes + } + + fn handle_link_control_2_write(&mut self, val: u32) { + // Link Control 2 (2 bytes) + Link Status 2 (2 bytes) + let new_link_control_2 = pci_express::LinkControl2::from_bits(val as u16); + let mut state = self.state.lock(); + + // Validate that target_link_speed doesn't exceed max_link_speed from Link Capabilities + let max_speed = self.link_capabilities.max_link_speed(); + let requested_speed = new_link_control_2.target_link_speed(); + + // Clamp the target link speed to not exceed the maximum supported speed + let actual_speed = if requested_speed > max_speed as u16 { + max_speed as u16 + } else { + requested_speed + }; + + // Update Link Control 2 with the validated speed + state.link_control_2 = new_link_control_2.with_target_link_speed(actual_speed); + + // Update Link Status to reflect the target link speed as current link speed + // This simulates the link retraining and speed negotiation completing immediately + state.link_status = state.link_status.with_current_link_speed(actual_speed); + + // Link Status 2 upper 16 bits - mostly read-only, so we don't modify it + } + + /// Enable hotplug support for this PCIe capability. + /// This configures the appropriate registers to support hotpluggable devices. + /// Panics if called on device types other than RootPort or DownstreamSwitchPort. + /// + /// # Arguments + /// * `slot_number` - The physical slot number to assign to this hotplug-capable port + pub fn with_hotplug_support(mut self, slot_number: u32) -> Self { + use pci_express::DevicePortType; + + // Validate that hotplug is only enabled for appropriate port types + let port_type = self.pcie_capabilities.device_port_type(); + match port_type { + DevicePortType::RootPort | DevicePortType::DownstreamSwitchPort => { + // Valid port types for hotplug support + } + DevicePortType::Endpoint | DevicePortType::UpstreamSwitchPort => { + panic!( + "Hotplug support is not valid for device port type {:?}. \ + Only RootPort and DownstreamSwitchPort support hotplug.", + port_type + ); + } + } + + // Enable slot implemented in PCIe capabilities when hotplug is enabled + self.pcie_capabilities = self.pcie_capabilities.with_slot_implemented(true); + + // Enable hotplug capabilities in slot capabilities register + self.slot_capabilities = self + .slot_capabilities + .with_hot_plug_surprise(true) + .with_hot_plug_capable(true) + .with_physical_slot_number(slot_number); + + // Enable Data Link Layer Link Active Reporting when hotplug is enabled + self.link_capabilities = self + .link_capabilities + .with_data_link_layer_link_active_reporting(true); + + self + } + + /// Set the presence detect state for the slot. + /// This method only has effect if the slot is implemented (slot_implemented = true). + /// If slot is not implemented, the call is silently ignored, as the spec says + /// "If this register is implemented but the Slot Implemented bit is Clear, + /// the field behavior of this entire register with the exception of the DLLSC bit is undefined." + /// + /// # Arguments + /// * `present` - true if a device is present in the slot, false if the slot is empty + pub fn set_presence_detect_state(&self, present: bool) { + if !self.pcie_capabilities.slot_implemented() { + // Silently ignore if slot is not implemented + return; + } + + let mut state = self.state.lock(); + state.slot_status = + state + .slot_status + .with_presence_detect_state(if present { 1 } else { 0 }); } } @@ -125,6 +339,10 @@ impl PciCapability for PciExpressCapability { "pci-express" } + fn capability_id(&self) -> CapabilityId { + CapabilityId::PCI_EXPRESS + } + fn len(&self) -> usize { // Implement the full PCI Express Capability structure (PCI Spec, Section 7.5.3): // 0x00: PCIe Capabilities (2 bytes) + Next Pointer (1 byte) + Capability ID (1 byte) @@ -240,31 +458,7 @@ impl PciCapability for PciExpressCapability { ); } PciExpressCapabilityHeader::DEVICE_CTL_STS => { - // Lower 16 bits are Device Control (read-write) - // Upper 16 bits are Device Status (read-write, but some bits are read-only) - let new_control = pci_express::DeviceControl::from_bits(val as u16); - self.handle_device_control_write(new_control); - - // Handle Device Status - most bits are write-1-to-clear - let new_status = pci_express::DeviceStatus::from_bits((val >> 16) as u16); - let mut state = self.state.lock(); - let mut current_status = state.device_status; - - // Clear bits that were written as 1 (write-1-to-clear semantics) - if new_status.correctable_error_detected() { - current_status.set_correctable_error_detected(false); - } - if new_status.non_fatal_error_detected() { - current_status.set_non_fatal_error_detected(false); - } - if new_status.fatal_error_detected() { - current_status.set_fatal_error_detected(false); - } - if new_status.unsupported_request_detected() { - current_status.set_unsupported_request_detected(false); - } - - state.device_status = current_status; + self.handle_device_control_status_write(val); } PciExpressCapabilityHeader::LINK_CAPS => { // Link Capabilities register is read-only @@ -276,11 +470,7 @@ impl PciCapability for PciExpressCapability { ); } PciExpressCapabilityHeader::LINK_CTL_STS => { - // Link Control (2 bytes) + Link Status (2 bytes) - let mut state = self.state.lock(); - state.link_control = pci_express::LinkControl::from_bits(val as u16); - // Link Status upper 16 bits - many bits are write-1-to-clear or read-only - // For simplicity, we'll treat it as read-only for now + self.handle_link_control_status_write(val); } PciExpressCapabilityHeader::SLOT_CAPS => { // Slot Capabilities register is read-only @@ -292,12 +482,7 @@ impl PciCapability for PciExpressCapability { ); } PciExpressCapabilityHeader::SLOT_CTL_STS => { - // Slot Control (2 bytes) + Slot Status (2 bytes) - let mut state = self.state.lock(); - state.slot_control = pci_express::SlotControl::from_bits(val as u16); - // Slot Status upper 16 bits - many bits are write-1-to-clear - // For simplicity, we'll allow basic writes for now - state.slot_status = pci_express::SlotStatus::from_bits((val >> 16) as u16); + self.handle_slot_control_status_write(val); } PciExpressCapabilityHeader::ROOT_CTL_CAPS => { // Root Control (2 bytes) + Root Capabilities (2 bytes) @@ -337,10 +522,7 @@ impl PciCapability for PciExpressCapability { ); } PciExpressCapabilityHeader::LINK_CTL_STS_2 => { - // Link Control 2 (2 bytes) + Link Status 2 (2 bytes) - let mut state = self.state.lock(); - state.link_control_2 = pci_express::LinkControl2::from_bits(val as u16); - // Link Status 2 upper 16 bits - mostly read-only + self.handle_link_control_2_write(val); } PciExpressCapabilityHeader::SLOT_CAPS_2 => { // Slot Capabilities 2 register is read-only @@ -373,6 +555,14 @@ impl PciCapability for PciExpressCapability { let mut state = self.state.lock(); *state = PciExpressState::new(); } + + fn as_pci_express(&self) -> Option<&PciExpressCapability> { + Some(self) + } + + fn as_pci_express_mut(&mut self) -> Option<&mut PciExpressCapability> { + Some(self) + } } mod save_restore { @@ -472,9 +662,11 @@ mod tests { let device_ctl_sts_val = cap.read_u32(0x08); assert_eq!(device_ctl_sts_val, 0); // Both control and status should be 0 - // Test unhandled offset - should return 0 and not panic - let unhandled_val = cap.read_u32(0x10); - assert_eq!(unhandled_val, 0); + // Test Link Control/Status Register (offset 0x10) - should have link status initialized + let link_ctl_sts_val = cap.read_u32(0x10); + let expected_link_status = (LinkSpeed::Speed32_0GtS.into_bits() as u16) + | ((LinkWidth::X16.into_bits() as u16) << 4); // current_link_speed + negotiated_link_width + assert_eq!(link_ctl_sts_val, (expected_link_status as u32) << 16); // Link status is in upper 16 bits } #[test] @@ -618,17 +810,27 @@ mod tests { fn test_pci_express_capability_extended_registers() { let cap = PciExpressCapability::new(DevicePortType::Endpoint, None); - // Test that extended registers return 0 by default and don't crash - assert_eq!(cap.read_u32(0x0C), 0); // Link Capabilities - assert_eq!(cap.read_u32(0x10), 0); // Link Control/Status + // Test that extended registers return proper default values and don't crash + // Link Capabilities should have default speed (Speed32_0GtS) and width (X16) + let expected_link_caps = + LinkSpeed::Speed32_0GtS.into_bits() | (LinkWidth::X16.into_bits() << 4); // speed + (width << 4) = 5 + (16 << 4) = 5 + 256 = 261 + assert_eq!(cap.read_u32(0x0C), expected_link_caps); // Link Capabilities + // Link Control/Status should have Link Status with current_link_speed=5 and negotiated_link_width=16 + let expected_link_ctl_sts = (LinkSpeed::Speed32_0GtS.into_bits() as u16) + | ((LinkWidth::X16.into_bits() as u16) << 4); // current_link_speed (bits 0-3) + negotiated_link_width (bits 4-9) = 5 + (16 << 4) = 5 + 256 = 261 + assert_eq!(cap.read_u32(0x10), (expected_link_ctl_sts as u32) << 16); // Link Control/Status (status in upper 16 bits) assert_eq!(cap.read_u32(0x14), 0); // Slot Capabilities assert_eq!(cap.read_u32(0x18), 0); // Slot Control/Status assert_eq!(cap.read_u32(0x1C), 0); // Root Control/Capabilities assert_eq!(cap.read_u32(0x20), 0); // Root Status assert_eq!(cap.read_u32(0x24), 0); // Device Capabilities 2 assert_eq!(cap.read_u32(0x28), 0); // Device Control/Status 2 - assert_eq!(cap.read_u32(0x2C), 0); // Link Capabilities 2 - assert_eq!(cap.read_u32(0x30), 0); // Link Control/Status 2 + // Link Capabilities 2 has supported_link_speeds_vector set to UpToGen5 + let expected_link_caps_2 = SupportedLinkSpeedsVector::UpToGen5.into_bits() << 1; // supported_link_speeds_vector at bits 1-7 = 31 << 1 = 62 + assert_eq!(cap.read_u32(0x2C), expected_link_caps_2); // Link Capabilities 2 + // Link Control/Status 2 - Link Control 2 should have target_link_speed set to Speed32_0GtS (5) + let expected_link_ctl_sts_2 = LinkSpeed::Speed32_0GtS.into_bits() as u16; // target_link_speed in lower 4 bits = 5 + assert_eq!(cap.read_u32(0x30), expected_link_ctl_sts_2 as u32); // Link Control/Status 2 assert_eq!(cap.read_u32(0x34), 0); // Slot Capabilities 2 assert_eq!(cap.read_u32(0x38), 0); // Slot Control/Status 2 } @@ -644,4 +846,607 @@ mod tests { let cap = PciExpressCapability::new(DevicePortType::Endpoint, None); assert_eq!(cap.label(), "pci-express"); } + + #[test] + fn test_pci_express_capability_with_hotplug_support() { + // Test with RootPort (should work) + let cap = PciExpressCapability::new(DevicePortType::RootPort, None); + let cap_with_hotplug = cap.with_hotplug_support(1); + + // Verify that the method doesn't crash and returns the capability + assert_eq!(cap_with_hotplug.label(), "pci-express"); + assert_eq!(cap_with_hotplug.len(), 0x3C); + + // Verify hotplug capabilities are set + assert!(cap_with_hotplug.slot_capabilities.hot_plug_surprise()); + assert!(cap_with_hotplug.slot_capabilities.hot_plug_capable()); + assert_eq!(cap_with_hotplug.slot_capabilities.physical_slot_number(), 1); + + // Verify that slot_implemented is set in PCIe capabilities + assert!( + cap_with_hotplug.pcie_capabilities.slot_implemented(), + "slot_implemented should be true when hotplug is enabled" + ); + + // Test with DownstreamSwitchPort (should work) + let cap2 = PciExpressCapability::new(DevicePortType::DownstreamSwitchPort, None); + let cap2_with_hotplug = cap2.with_hotplug_support(2); + + assert!(cap2_with_hotplug.slot_capabilities.hot_plug_surprise()); + assert!(cap2_with_hotplug.slot_capabilities.hot_plug_capable()); + assert_eq!( + cap2_with_hotplug.slot_capabilities.physical_slot_number(), + 2 + ); + + // Verify that slot_implemented is set for downstream switch port too + assert!( + cap2_with_hotplug.pcie_capabilities.slot_implemented(), + "slot_implemented should be true when hotplug is enabled" + ); + + // Test that non-hotplug capability doesn't have slot_implemented set + let cap_no_hotplug = PciExpressCapability::new(DevicePortType::RootPort, None); + assert!( + !cap_no_hotplug.pcie_capabilities.slot_implemented(), + "slot_implemented should be false when hotplug is not enabled" + ); + } + + #[test] + #[should_panic(expected = "Hotplug support is not valid for device port type Endpoint")] + fn test_pci_express_capability_with_hotplug_support_endpoint_panics() { + let cap = PciExpressCapability::new(DevicePortType::Endpoint, None); + cap.with_hotplug_support(1); + } + + #[test] + #[should_panic( + expected = "Hotplug support is not valid for device port type UpstreamSwitchPort" + )] + fn test_pci_express_capability_with_hotplug_support_upstream_panics() { + let cap = PciExpressCapability::new(DevicePortType::UpstreamSwitchPort, None); + cap.with_hotplug_support(1); + } + + #[test] + fn test_slot_control_write_protection() { + // Create a root port capability with hotplug support but limited slot capabilities + let mut cap = PciExpressCapability::new(DevicePortType::RootPort, None); + cap = cap.with_hotplug_support(1); + + // Modify slot capabilities to disable some features for testing + cap.slot_capabilities.set_attention_button_present(false); + cap.slot_capabilities.set_power_controller_present(false); + cap.slot_capabilities.set_mrl_sensor_present(false); + cap.slot_capabilities.set_attention_indicator_present(false); + cap.slot_capabilities.set_power_indicator_present(false); + cap.slot_capabilities + .set_electromechanical_interlock_present(false); + cap.slot_capabilities.set_no_command_completed_support(true); + + // Try to write to slot control register with all bits set + let slot_ctl_sts_offset = 0x18; // SLOT_CTL_STS offset + let val_to_write = 0xFFFFFFFF; // All bits set in both control and status + + cap.write_u32(slot_ctl_sts_offset, val_to_write); + + // Read back the slot control register (lower 16 bits) + let read_back = cap.read_u32(slot_ctl_sts_offset); + let slot_control_value = read_back as u16; + let slot_control = pci_express::SlotControl::from_bits(slot_control_value); + + // Verify that features not present in capabilities were not set in control register + assert!( + !slot_control.attention_button_pressed_enable(), + "Attention button enable should be 0 when capability not present" + ); + assert!( + !slot_control.power_controller_control(), + "Power controller control should be 0 when capability not present" + ); + assert!( + !slot_control.mrl_sensor_changed_enable(), + "MRL sensor changed enable should be 0 when capability not present" + ); + assert_eq!( + slot_control.attention_indicator_control(), + 0, + "Attention indicator control should be 0 when capability not present" + ); + assert_eq!( + slot_control.power_indicator_control(), + 0, + "Power indicator control should be 0 when capability not present" + ); + assert!( + !slot_control.electromechanical_interlock_control(), + "Electromechanical interlock control should be 0 when capability not present" + ); + assert!( + !slot_control.command_completed_interrupt_enable(), + "Command completed interrupt enable should be 0 when no command completed support" + ); + + // However, hotplug interrupt enable should be settable since hotplug is capable + assert!( + slot_control.hot_plug_interrupt_enable(), + "Hotplug interrupt enable should be settable when hotplug capable" + ); + } + + #[test] + fn test_link_control_retrain_link_behavior() { + // Test that retrain_link always reads as 0 regardless of what is written + let mut cap = PciExpressCapability::new(DevicePortType::RootPort, None); + + let link_ctl_sts_offset = 0x10; // LINK_CTL_STS offset + + // Write a value with retrain_link bit set (bit 5) + let write_val = 0x0020; // retrain_link bit (bit 5) = 1 + cap.write_u32(link_ctl_sts_offset, write_val); + + // Read back and verify retrain_link is always 0 + let read_back = cap.read_u32(link_ctl_sts_offset); + let link_control = pci_express::LinkControl::from_bits(read_back as u16); + + assert!( + !link_control.retrain_link(), + "retrain_link should always read as 0" + ); + + // Verify other bits can still be set (except retrain_link) + let write_val_2 = 0x0001; // aspm_control bit 0 = 1 + cap.write_u32(link_ctl_sts_offset, write_val_2); + + let read_back_2 = cap.read_u32(link_ctl_sts_offset); + let link_control_2 = pci_express::LinkControl::from_bits(read_back_2 as u16); + + assert_eq!( + link_control_2.aspm_control(), + 1, + "Other control bits should be settable" + ); + assert!( + !link_control_2.retrain_link(), + "retrain_link should still read as 0" + ); + } + + #[test] + fn test_hotplug_link_capabilities() { + // Test that Data Link Layer Link Active Reporting is enabled with hotplug + let cap = PciExpressCapability::new(DevicePortType::RootPort, None); + let cap_with_hotplug = cap.with_hotplug_support(1); + + let link_caps_offset = 0x0C; // LINK_CAPS offset + let link_caps = cap_with_hotplug.read_u32(link_caps_offset); + let link_capabilities = pci_express::LinkCapabilities::from_bits(link_caps); + + // Verify that Data Link Layer Link Active Reporting is enabled + assert!( + link_capabilities.data_link_layer_link_active_reporting(), + "Data Link Layer Link Active Reporting should be enabled for hotplug" + ); + + // Verify default speed and width are still correct + assert_eq!( + link_capabilities.max_link_speed(), + LinkSpeed::Speed32_0GtS.into_bits(), + "Max link speed should be Speed32_0GtS (PCIe 32.0 GT/s)" + ); + assert_eq!( + link_capabilities.max_link_width(), + LinkWidth::X16.into_bits(), + "Max link width should be X16 (x16)" + ); + + // Test that non-hotplug capability doesn't have Data Link Layer Link Active Reporting + let cap_no_hotplug = PciExpressCapability::new(DevicePortType::RootPort, None); + let link_caps_no_hotplug = cap_no_hotplug.read_u32(link_caps_offset); + let link_capabilities_no_hotplug = + pci_express::LinkCapabilities::from_bits(link_caps_no_hotplug); + + assert!( + !link_capabilities_no_hotplug.data_link_layer_link_active_reporting(), + "Data Link Layer Link Active Reporting should be disabled without hotplug" + ); + } + + #[test] + fn test_link_status_read_only() { + // Test that Link Status register is read-only and cannot be modified by writes + let mut cap = PciExpressCapability::new(DevicePortType::RootPort, None); + + let link_ctl_sts_offset = 0x10; // LINK_CTL_STS offset + + // Set some initial link status values (this would normally be done by hardware) + { + let mut state = cap.state.lock(); + state.link_status.set_current_link_speed(0b0001); // Set initial speed + state.link_status.set_negotiated_link_width(0b000001); // Set initial width + state.link_status.set_link_training(true); // Set link training active + state.link_status.set_data_link_layer_link_active(true); // Set DLL active + } + + // Read initial values + let initial_read = cap.read_u32(link_ctl_sts_offset); + let initial_link_status = pci_express::LinkStatus::from_bits((initial_read >> 16) as u16); + + // Verify initial values are set + assert_eq!( + initial_link_status.current_link_speed(), + 0b0001, + "Initial link speed should be set" + ); + assert_eq!( + initial_link_status.negotiated_link_width(), + 0b000001, + "Initial link width should be set" + ); + assert!( + initial_link_status.link_training(), + "Initial link training should be active" + ); + assert!( + initial_link_status.data_link_layer_link_active(), + "Initial DLL should be active" + ); + + // Try to write different values to Link Status (upper 16 bits) while also writing to Link Control + let write_val = 0xFFFF0001; // Upper 16 bits all 1s (Link Status), lower 16 bits = 1 (Link Control) + cap.write_u32(link_ctl_sts_offset, write_val); + + // Read back and verify Link Status hasn't changed + let after_write = cap.read_u32(link_ctl_sts_offset); + let final_link_status = pci_express::LinkStatus::from_bits((after_write >> 16) as u16); + let final_link_control = pci_express::LinkControl::from_bits(after_write as u16); + + // Link Status should remain unchanged (read-only) + assert_eq!( + final_link_status.current_link_speed(), + initial_link_status.current_link_speed(), + "Link Status current_link_speed should be read-only" + ); + assert_eq!( + final_link_status.negotiated_link_width(), + initial_link_status.negotiated_link_width(), + "Link Status negotiated_link_width should be read-only" + ); + assert_eq!( + final_link_status.link_training(), + initial_link_status.link_training(), + "Link Status link_training should be read-only" + ); + assert_eq!( + final_link_status.data_link_layer_link_active(), + initial_link_status.data_link_layer_link_active(), + "Link Status data_link_layer_link_active should be read-only" + ); + + // But Link Control should be modifiable + assert_eq!( + final_link_control.aspm_control(), + 1, + "Link Control should be writable" + ); + } + + #[test] + fn test_slot_status_rw1c_behavior() { + // Create a root port capability with hotplug support + let mut cap = PciExpressCapability::new(DevicePortType::RootPort, None); + cap = cap.with_hotplug_support(1); + + let slot_ctl_sts_offset = 0x18; // SLOT_CTL_STS offset + + // First, simulate setting some status bits (this would normally be done by hardware) + { + let mut state = cap.state.lock(); + state.slot_status.set_attention_button_pressed(true); + state.slot_status.set_power_fault_detected(true); + state.slot_status.set_mrl_sensor_changed(true); + state.slot_status.set_presence_detect_changed(true); + state.slot_status.set_command_completed(true); + state.slot_status.set_data_link_layer_state_changed(true); + // Set some RO bits too + state.slot_status.set_mrl_sensor_state(1); + state.slot_status.set_presence_detect_state(1); + state.slot_status.set_electromechanical_interlock_status(1); + } + + // Read the initial status to verify all bits are set + let initial_read = cap.read_u32(slot_ctl_sts_offset); + let initial_status = pci_express::SlotStatus::from_bits((initial_read >> 16) as u16); + assert!( + initial_status.attention_button_pressed(), + "Initial attention button pressed should be set" + ); + assert!( + initial_status.power_fault_detected(), + "Initial power fault detected should be set" + ); + assert!( + initial_status.mrl_sensor_changed(), + "Initial MRL sensor changed should be set" + ); + assert!( + initial_status.presence_detect_changed(), + "Initial presence detect changed should be set" + ); + assert!( + initial_status.command_completed(), + "Initial command completed should be set" + ); + assert!( + initial_status.data_link_layer_state_changed(), + "Initial data link layer state changed should be set" + ); + assert_eq!( + initial_status.mrl_sensor_state(), + 1, + "Initial MRL sensor state should be set" + ); + assert_eq!( + initial_status.presence_detect_state(), + 1, + "Initial presence detect state should be set" + ); + assert_eq!( + initial_status.electromechanical_interlock_status(), + 1, + "Initial electromechanical interlock status should be set" + ); + + // Write 1 to clear specific RW1C bits (upper 16 bits contain status) + // Write 1s only to some RW1C bits to test selective clearing + // Bit positions: attention_button_pressed(0), command_completed(4), data_link_layer_state_changed(8) + let write_val = (0b0000_0001_0001_0001_u16 as u32) << 16; // Clear bits 0, 4, and 8 + cap.write_u32(slot_ctl_sts_offset, write_val); + + // Read back and verify RW1C behavior + let after_write = cap.read_u32(slot_ctl_sts_offset); + let final_status = pci_express::SlotStatus::from_bits((after_write >> 16) as u16); + + // RW1C bits that were written with 1 should be cleared + assert!( + !final_status.attention_button_pressed(), + "Attention button pressed should be cleared after write-1" + ); + assert!( + !final_status.command_completed(), + "Command completed should be cleared after write-1" + ); + assert!( + !final_status.data_link_layer_state_changed(), + "Data link layer state changed should be cleared after write-1" + ); + + // RW1C bits that were written with 0 should remain unchanged + assert!( + final_status.power_fault_detected(), + "Power fault detected should remain set (write-0)" + ); + assert!( + final_status.mrl_sensor_changed(), + "MRL sensor changed should remain set (write-0)" + ); + assert!( + final_status.presence_detect_changed(), + "Presence detect changed should remain set (write-0)" + ); + + // RO bits should remain unchanged regardless of what was written + assert_eq!( + final_status.mrl_sensor_state(), + 1, + "MRL sensor state should remain unchanged (RO)" + ); + assert_eq!( + final_status.presence_detect_state(), + 1, + "Presence detect state should remain unchanged (RO)" + ); + assert_eq!( + final_status.electromechanical_interlock_status(), + 1, + "Electromechanical interlock status should remain unchanged (RO)" + ); + } + + #[test] + fn test_link_control_2_target_speed_validation() { + // Test that target link speed is validated against max link speed and reflected in link status + let mut cap = PciExpressCapability::new(DevicePortType::RootPort, None); + + let link_ctl_sts_2_offset = 0x30; // LINK_CTL_STS_2 offset + + // Initially, target link speed should be Speed32_0GtS (5) and current link speed should match + let initial_read = cap.read_u32(link_ctl_sts_2_offset); + let initial_link_control_2 = pci_express::LinkControl2::from_bits(initial_read as u16); + assert_eq!( + initial_link_control_2.target_link_speed(), + LinkSpeed::Speed32_0GtS.into_bits() as u16, + "Initial target link speed should be Speed32_0GtS" + ); + + // Check that link status reflects this speed + let link_ctl_sts_offset = 0x10; // LINK_CTL_STS offset + let link_ctl_sts = cap.read_u32(link_ctl_sts_offset); + let link_status = pci_express::LinkStatus::from_bits((link_ctl_sts >> 16) as u16); + assert_eq!( + link_status.current_link_speed(), + LinkSpeed::Speed32_0GtS.into_bits() as u16, + "Initial current link speed should match target speed" + ); + assert_eq!( + link_status.negotiated_link_width(), + LinkWidth::X16.into_bits() as u16, + "Initial negotiated link width should be X16" + ); + + // Test writing a valid speed (Speed16_0GtS = 4) that's less than max speed (Speed32_0GtS = 5) + let valid_speed = LinkSpeed::Speed16_0GtS.into_bits() as u16; // 4 + cap.write_u32(link_ctl_sts_2_offset, valid_speed as u32); + + // Verify target link speed was set correctly + let after_valid_write = cap.read_u32(link_ctl_sts_2_offset); + let link_control_2_after_valid = + pci_express::LinkControl2::from_bits(after_valid_write as u16); + assert_eq!( + link_control_2_after_valid.target_link_speed(), + valid_speed, + "Target link speed should be set to requested valid speed" + ); + + // Verify current link speed was updated in link status + let link_ctl_sts_after_valid = cap.read_u32(link_ctl_sts_offset); + let link_status_after_valid = + pci_express::LinkStatus::from_bits((link_ctl_sts_after_valid >> 16) as u16); + assert_eq!( + link_status_after_valid.current_link_speed(), + valid_speed, + "Current link speed should be updated to match target speed" + ); + + // Test writing an invalid speed (Speed64_0GtS = 6) that exceeds max speed (Speed32_0GtS = 5) + let invalid_speed = LinkSpeed::Speed64_0GtS.into_bits() as u16; // 6 + cap.write_u32(link_ctl_sts_2_offset, invalid_speed as u32); + + // Verify target link speed was clamped to max speed + let after_invalid_write = cap.read_u32(link_ctl_sts_2_offset); + let link_control_2_after_invalid = + pci_express::LinkControl2::from_bits(after_invalid_write as u16); + let max_speed = LinkSpeed::Speed32_0GtS.into_bits() as u16; // 5 + assert_eq!( + link_control_2_after_invalid.target_link_speed(), + max_speed, + "Target link speed should be clamped to max supported speed" + ); + + // Verify current link speed was updated to the clamped value + let link_ctl_sts_after_invalid = cap.read_u32(link_ctl_sts_offset); + let link_status_after_invalid = + pci_express::LinkStatus::from_bits((link_ctl_sts_after_invalid >> 16) as u16); + assert_eq!( + link_status_after_invalid.current_link_speed(), + max_speed, + "Current link speed should be updated to clamped max speed" + ); + + // Verify that link width remains unchanged throughout + assert_eq!( + link_status_after_valid.negotiated_link_width(), + LinkWidth::X16.into_bits() as u16, + "Negotiated link width should remain unchanged" + ); + assert_eq!( + link_status_after_invalid.negotiated_link_width(), + LinkWidth::X16.into_bits() as u16, + "Negotiated link width should remain unchanged" + ); + } + + #[test] + fn test_with_hotplug_support_slot_number() { + // Test that slot numbers are properly set when enabling hotplug support + + // Test with slot number 5 + let cap1 = PciExpressCapability::new(DevicePortType::RootPort, None); + let cap1_with_hotplug = cap1.with_hotplug_support(5); + + assert!(cap1_with_hotplug.slot_capabilities.hot_plug_capable()); + assert_eq!( + cap1_with_hotplug.slot_capabilities.physical_slot_number(), + 5 + ); + + // Test with slot number 0 + let cap2 = PciExpressCapability::new(DevicePortType::DownstreamSwitchPort, None); + let cap2_with_hotplug = cap2.with_hotplug_support(0); + + assert!(cap2_with_hotplug.slot_capabilities.hot_plug_capable()); + assert_eq!( + cap2_with_hotplug.slot_capabilities.physical_slot_number(), + 0 + ); + + // Test with a larger slot number + let cap3 = PciExpressCapability::new(DevicePortType::RootPort, None); + let cap3_with_hotplug = cap3.with_hotplug_support(255); + + assert!(cap3_with_hotplug.slot_capabilities.hot_plug_capable()); + assert_eq!( + cap3_with_hotplug.slot_capabilities.physical_slot_number(), + 255 + ); + } + + #[test] + fn test_slot_implemented_flag_in_pcie_capabilities_register() { + // Test that slot_implemented bit is correctly set in the PCIe Capabilities register + // when hotplug support is enabled + + // Test without hotplug - slot_implemented should be false + let cap_no_hotplug = PciExpressCapability::new(DevicePortType::RootPort, None); + let caps_val_no_hotplug = cap_no_hotplug.read_u32(0x00); + let pcie_caps_no_hotplug = (caps_val_no_hotplug >> 16) as u16; + let slot_implemented_bit = (pcie_caps_no_hotplug >> 8) & 0x1; // slot_implemented is bit 8 of PCIe capabilities + assert_eq!( + slot_implemented_bit, 0, + "slot_implemented should be 0 when hotplug is not enabled" + ); + + // Test with hotplug - slot_implemented should be true + let cap_with_hotplug = cap_no_hotplug.with_hotplug_support(1); + let caps_val_with_hotplug = cap_with_hotplug.read_u32(0x00); + let pcie_caps_with_hotplug = (caps_val_with_hotplug >> 16) as u16; + let slot_implemented_bit_hotplug = (pcie_caps_with_hotplug >> 8) & 0x1; // slot_implemented is bit 8 of PCIe capabilities + assert_eq!( + slot_implemented_bit_hotplug, 1, + "slot_implemented should be 1 when hotplug is enabled" + ); + } + + #[test] + fn test_set_presence_detect_state() { + // Test setting presence detect state on a hotplug-capable port + let cap = PciExpressCapability::new(DevicePortType::RootPort, None).with_hotplug_support(1); + + // Initially, presence detect state should be 0 (no device present) + let initial_slot_status = cap.read_u32(0x18); // Slot Control + Slot Status + let initial_presence_detect = (initial_slot_status >> 22) & 0x1; // presence_detect_state is bit 6 of slot status (upper 16 bits) + assert_eq!( + initial_presence_detect, 0, + "Initial presence detect state should be 0" + ); + + // Set device as present + cap.set_presence_detect_state(true); + let present_slot_status = cap.read_u32(0x18); + let present_presence_detect = (present_slot_status >> 22) & 0x1; + assert_eq!( + present_presence_detect, 1, + "Presence detect state should be 1 when device is present" + ); + + // Set device as not present + cap.set_presence_detect_state(false); + let absent_slot_status = cap.read_u32(0x18); + let absent_presence_detect = (absent_slot_status >> 22) & 0x1; + assert_eq!( + absent_presence_detect, 0, + "Presence detect state should be 0 when device is not present" + ); + } + + #[test] + fn test_set_presence_detect_state_without_slot_implemented() { + // Test that setting presence detect state is silently ignored when slot is not implemented + let cap = PciExpressCapability::new(DevicePortType::RootPort, None); + + // Should not panic and should be silently ignored + cap.set_presence_detect_state(true); + cap.set_presence_detect_state(false); + } } diff --git a/vm/devices/pci/pci_core/src/capabilities/read_only.rs b/vm/devices/pci/pci_core/src/capabilities/read_only.rs index 9d50571bf6..f5f1487ab8 100644 --- a/vm/devices/pci/pci_core/src/capabilities/read_only.rs +++ b/vm/devices/pci/pci_core/src/capabilities/read_only.rs @@ -4,6 +4,7 @@ //! A generic, read-only PCI Capability (backed by an [`IntoBytes`] type). use super::PciCapability; +use crate::spec::caps::CapabilityId; use inspect::Inspect; use std::fmt::Debug; use zerocopy::Immutable; @@ -14,14 +15,29 @@ use zerocopy::KnownLayout; #[derive(Debug)] pub struct ReadOnlyCapability { label: String, + capability_id: CapabilityId, data: T, } impl ReadOnlyCapability { - /// Create a new [`ReadOnlyCapability`] + /// Create a new [`ReadOnlyCapability`] with VENDOR_SPECIFIC capability ID pub fn new(label: impl Into, data: T) -> Self { Self { label: label.into(), + capability_id: CapabilityId::VENDOR_SPECIFIC, + data, + } + } + + /// Create a new [`ReadOnlyCapability`] with a specific capability ID + pub fn new_with_capability_id( + label: impl Into, + capability_id: CapabilityId, + data: T, + ) -> Self { + Self { + label: label.into(), + capability_id, data, } } @@ -31,18 +47,23 @@ impl Inspect for ReadOnlyCapability { fn inspect(&self, req: inspect::Request<'_>) { req.respond() .field("label", &self.label) + .field("capability_id", format!("0x{:02X}", self.capability_id.0)) .display_debug("data", &self.data); } } impl PciCapability for ReadOnlyCapability where - T: IntoBytes + Send + Sync + Debug + Immutable + KnownLayout, + T: IntoBytes + Send + Sync + Debug + Immutable + KnownLayout + 'static, { fn label(&self) -> &str { &self.label } + fn capability_id(&self) -> CapabilityId { + self.capability_id + } + fn len(&self) -> usize { size_of::() } diff --git a/vm/devices/pci/pci_core/src/cfg_space_emu.rs b/vm/devices/pci/pci_core/src/cfg_space_emu.rs index a720ec85c0..1c0347b7a2 100644 --- a/vm/devices/pci/pci_core/src/cfg_space_emu.rs +++ b/vm/devices/pci/pci_core/src/cfg_space_emu.rs @@ -9,6 +9,7 @@ use crate::PciInterruptPin; use crate::bar_mapping::BarMappings; use crate::capabilities::PciCapability; +use crate::spec::caps::CapabilityId; use crate::spec::cfg_space; use crate::spec::hwid::HardwareIds; use chipset_device::io::IoError; @@ -22,6 +23,28 @@ use std::sync::atomic::AtomicBool; use std::sync::atomic::Ordering; use vmcore::line_interrupt::LineInterrupt; +/// Result type for common header emulator operations +#[derive(Debug)] +pub enum CommonHeaderResult { + /// The access was handled by the common header emulator + Handled, + /// The access is not handled by common header, caller should handle it + Unhandled, + /// The access failed with an error + Failed(IoError), +} + +impl PartialEq for CommonHeaderResult { + fn eq(&self, other: &Self) -> bool { + match (self, other) { + (Self::Handled, Self::Handled) => true, + (Self::Unhandled, Self::Unhandled) => true, + (Self::Failed(_), Self::Failed(_)) => true, // Consider all failures equal for testing + _ => false, + } + } +} + const SUPPORTED_COMMAND_BITS: u16 = cfg_space::Command::new() .with_pio_enabled(true) .with_mmio_enabled(true) @@ -91,47 +114,45 @@ impl IntxInterrupt { } #[derive(Debug, Inspect)] -struct ConfigSpaceType0EmulatorState { +struct ConfigSpaceCommonHeaderEmulatorState { /// The command register command: cfg_space::Command, /// OS-configured BARs - #[inspect(with = "inspect_helpers::bars")] - base_addresses: [u32; 6], + #[inspect(with = "inspect_helpers::bars_generic")] + base_addresses: [u32; N], /// The PCI device doesn't actually care about what value is stored here - /// this register is just a bit of standardized "scratch space", ostensibly /// for firmware to communicate IRQ assignments to the OS, but it can really /// be used for just about anything. interrupt_line: u8, - /// A read/write register that doesn't matter in virtualized contexts - latency_timer: u8, } -impl ConfigSpaceType0EmulatorState { +impl ConfigSpaceCommonHeaderEmulatorState { fn new() -> Self { Self { - latency_timer: 0, command: cfg_space::Command::new(), - base_addresses: [0; 6], + base_addresses: { + const ZERO: u32 = 0; + [ZERO; N] + }, interrupt_line: 0, } } } -/// Emulator for the standard Type 0 PCI configuration space header. -// -// TODO: Figure out how to split this up and share the handling of common -// registers (hardware IDs, command, status, etc.) with the type 1 emulator. +/// Common emulator for shared PCI configuration space functionality. +/// Generic over the number of BARs (6 for Type 0, 2 for Type 1). #[derive(Inspect)] -pub struct ConfigSpaceType0Emulator { +pub struct ConfigSpaceCommonHeaderEmulator { // Fixed configuration - #[inspect(with = "inspect_helpers::bars")] - bar_masks: [u32; 6], + #[inspect(with = "inspect_helpers::bars_generic")] + bar_masks: [u32; N], hardware_ids: HardwareIds, multi_function_bit: bool, // Runtime glue #[inspect(with = r#"|x| inspect::iter_by_index(x).prefix("bar")"#)] - mapped_memory: [Option; 6], + mapped_memory: [Option; N], #[inspect(with = "|x| inspect::iter_by_key(x.iter().map(|cap| (cap.label(), cap)))")] capabilities: Vec>, intx_interrupt: Option>, @@ -140,13 +161,546 @@ pub struct ConfigSpaceType0Emulator { active_bars: BarMappings, // Volatile state + state: ConfigSpaceCommonHeaderEmulatorState, +} + +/// Type alias for Type 0 common header emulator (6 BARs) +pub type ConfigSpaceCommonHeaderEmulatorType0 = ConfigSpaceCommonHeaderEmulator<6>; + +/// Type alias for Type 1 common header emulator (2 BARs) +pub type ConfigSpaceCommonHeaderEmulatorType1 = ConfigSpaceCommonHeaderEmulator<2>; + +impl ConfigSpaceCommonHeaderEmulator { + /// Create a new common header emulator + pub fn new( + hardware_ids: HardwareIds, + capabilities: Vec>, + bars: DeviceBars, + ) -> Self { + let mut bar_masks = { + const ZERO: u32 = 0; + [ZERO; N] + }; + let mut mapped_memory = { + const NONE: Option = None; + [NONE; N] + }; + + // Only process BARs that fit within our supported range (N) + for (bar_index, bar) in bars.bars.into_iter().enumerate().take(N) { + let (len, mapped) = match bar { + Some(bar) => bar, + None => continue, + }; + // use 64-bit aware BARs + assert!(bar_index < N.saturating_sub(1)); + // Round up regions to a power of 2, as required by PCI (and + // inherently required by the BAR representation). Round up to at + // least one page to avoid various problems in guest OSes. + const MIN_BAR_SIZE: u64 = 4096; + let len = std::cmp::max(len.next_power_of_two(), MIN_BAR_SIZE); + let mask64 = !(len - 1); + bar_masks[bar_index] = cfg_space::BarEncodingBits::from_bits(mask64 as u32) + .with_type_64_bit(true) + .into_bits(); + if bar_index + 1 < N { + bar_masks[bar_index + 1] = (mask64 >> 32) as u32; + } + mapped_memory[bar_index] = Some(mapped); + } + + Self { + hardware_ids, + capabilities, + bar_masks, + mapped_memory, + multi_function_bit: false, + intx_interrupt: None, + active_bars: Default::default(), + state: ConfigSpaceCommonHeaderEmulatorState::new(), + } + } + + /// If the device is multi-function, enable bit 7 in the Header register. + pub fn with_multi_function_bit(mut self, bit: bool) -> Self { + self.multi_function_bit = bit; + self + } + + /// If using legacy INT#x interrupts: wire a LineInterrupt to one of the 4 + /// INT#x pins, returning an object that manages configuration space bits + /// when the device sets the interrupt level. + pub fn set_interrupt_pin( + &mut self, + pin: PciInterruptPin, + line: LineInterrupt, + ) -> Arc { + let intx_interrupt = Arc::new(IntxInterrupt { + pin, + line, + interrupt_disabled: AtomicBool::new(false), + interrupt_status: AtomicBool::new(false), + }); + self.intx_interrupt = Some(intx_interrupt.clone()); + intx_interrupt + } + + /// Reset the common header state + pub fn reset(&mut self) { + tracing::info!("ConfigSpaceCommonHeaderEmulator: resetting state"); + self.state = ConfigSpaceCommonHeaderEmulatorState::new(); + + tracing::info!("ConfigSpaceCommonHeaderEmulator: syncing command register after reset"); + self.sync_command_register(self.state.command); + + tracing::info!( + "ConfigSpaceCommonHeaderEmulator: resetting {} capabilities", + self.capabilities.len() + ); + for cap in &mut self.capabilities { + cap.reset(); + } + + if let Some(intx) = &mut self.intx_interrupt { + tracing::info!("ConfigSpaceCommonHeaderEmulator: resetting interrupt level"); + intx.set_level(false); + } + tracing::info!("ConfigSpaceCommonHeaderEmulator: reset completed"); + } + + /// Get hardware IDs + pub fn hardware_ids(&self) -> &HardwareIds { + &self.hardware_ids + } + + /// Get capabilities + pub fn capabilities(&self) -> &[Box] { + &self.capabilities + } + + /// Get capabilities mutably + pub fn capabilities_mut(&mut self) -> &mut [Box] { + &mut self.capabilities + } + + /// Get multi-function bit + pub fn multi_function_bit(&self) -> bool { + self.multi_function_bit + } + + /// Get current command register state + pub fn command(&self) -> cfg_space::Command { + self.state.command + } + + /// Get current base addresses + pub fn base_addresses(&self) -> &[u32; N] { + &self.state.base_addresses + } + + /// Get current interrupt line + pub fn interrupt_line(&self) -> u8 { + self.state.interrupt_line + } + + /// Get current interrupt pin (returns the pin number + 1, or 0 if no pin configured) + pub fn interrupt_pin(&self) -> u8 { + if let Some(intx) = &self.intx_interrupt { + (intx.pin as u8) + 1 // PCI spec: 1=INTA, 2=INTB, 3=INTC, 4=INTD, 0=no interrupt + } else { + 0 // No interrupt pin configured + } + } + + /// Set interrupt line (for save/restore) + pub fn set_interrupt_line(&mut self, interrupt_line: u8) { + self.state.interrupt_line = interrupt_line; + } + + /// Set base addresses (for save/restore) + pub fn set_base_addresses(&mut self, base_addresses: &[u32; N]) { + self.state.base_addresses = *base_addresses; + } + + /// Set command register (for save/restore) + pub fn set_command(&mut self, command: cfg_space::Command) { + self.state.command = command; + } + + /// Sync command register changes by updating both interrupt and MMIO state + pub fn sync_command_register(&mut self, command: cfg_space::Command) { + tracing::info!( + "ConfigSpaceCommonHeaderEmulator: syncing command register - intx_disable={}, mmio_enabled={}", + command.intx_disable(), + command.mmio_enabled() + ); + self.update_intx_disable(command.intx_disable()); + self.update_mmio_enabled(command.mmio_enabled()); + } + + /// Update interrupt disable setting + pub fn update_intx_disable(&mut self, disabled: bool) { + tracing::info!( + "ConfigSpaceCommonHeaderEmulator: updating intx_disable={}", + disabled + ); + if let Some(intx_interrupt) = &self.intx_interrupt { + intx_interrupt.set_disabled(disabled) + } + } + + /// Update MMIO enabled setting and handle BAR mapping + pub fn update_mmio_enabled(&mut self, enabled: bool) { + tracing::info!( + "ConfigSpaceCommonHeaderEmulator: updating mmio_enabled={}", + enabled + ); + if enabled { + // Note that BarMappings expects 6 BARs. Pad with 0 for Type 1 (N=2) + // and use directly for Type 0 (N=6). + let mut full_base_addresses = [0u32; 6]; + let mut full_bar_masks = [0u32; 6]; + + // Copy our data into the first N positions + full_base_addresses[..N].copy_from_slice(&self.state.base_addresses[..N]); + full_bar_masks[..N].copy_from_slice(&self.bar_masks[..N]); + + self.active_bars = BarMappings::parse(&full_base_addresses, &full_bar_masks); + for (bar, mapping) in self.mapped_memory.iter_mut().enumerate() { + if let Some(mapping) = mapping { + let base = self.active_bars.get(bar as u8).expect("bar exists"); + match mapping.map_to_guest(base) { + Ok(_) => {} + Err(err) => { + tracelimit::error_ratelimited!( + error = &err as &dyn std::error::Error, + bar, + base, + "failed to map bar", + ) + } + } + } + } + } else { + self.active_bars = Default::default(); + for mapping in self.mapped_memory.iter_mut().flatten() { + mapping.unmap_from_guest(); + } + } + } + + // ===== Configuration Space Read/Write Functions ===== + + /// Read from the config space. `offset` must be 32-bit aligned. + /// Returns CommonHeaderResult indicating if handled, unhandled, or failed. + pub fn read_u32(&self, offset: u16, value: &mut u32) -> CommonHeaderResult { + use cfg_space::CommonHeader; + + tracing::trace!( + "ConfigSpaceCommonHeaderEmulator: read_u32 offset={:#x}", + offset + ); + + *value = match CommonHeader(offset) { + CommonHeader::DEVICE_VENDOR => { + (self.hardware_ids.device_id as u32) << 16 | self.hardware_ids.vendor_id as u32 + } + CommonHeader::STATUS_COMMAND => { + let mut status = + cfg_space::Status::new().with_capabilities_list(!self.capabilities.is_empty()); + + if let Some(intx_interrupt) = &self.intx_interrupt { + if intx_interrupt.interrupt_status.load(Ordering::SeqCst) { + status.set_interrupt_status(true); + } + } + + (status.into_bits() as u32) << 16 | self.state.command.into_bits() as u32 + } + CommonHeader::CLASS_REVISION => { + (u8::from(self.hardware_ids.base_class) as u32) << 24 + | (u8::from(self.hardware_ids.sub_class) as u32) << 16 + | (u8::from(self.hardware_ids.prog_if) as u32) << 8 + | self.hardware_ids.revision_id as u32 + } + CommonHeader::RESERVED_CAP_PTR => { + if self.capabilities.is_empty() { + 0 + } else { + 0x40 + } + } + // Capabilities space - handled by common emulator + _ if (0x40..0x100).contains(&offset) => { + return self.read_capabilities(offset, value); + } + // Extended capabilities space - handled by common emulator + _ if (0x100..0x1000).contains(&offset) => { + return self.read_extended_capabilities(offset, value); + } + // Check if this is a BAR read + _ if self.is_bar_offset(offset) => { + return self.read_bar(offset, value); + } + // Unhandled access - not part of common header, caller should handle + _ => { + return CommonHeaderResult::Unhandled; + } + }; + + tracing::trace!( + "ConfigSpaceCommonHeaderEmulator: read_u32 offset={:#x} -> value={:#x}", + offset, + *value + ); + // Handled access + CommonHeaderResult::Handled + } + + /// Write to the config space. `offset` must be 32-bit aligned. + /// Returns CommonHeaderResult indicating if handled, unhandled, or failed. + pub fn write_u32(&mut self, offset: u16, val: u32) -> CommonHeaderResult { + use cfg_space::CommonHeader; + + tracing::trace!( + "ConfigSpaceCommonHeaderEmulator: write_u32 offset={:#x} val={:#x}", + offset, + val + ); + + match CommonHeader(offset) { + CommonHeader::STATUS_COMMAND => { + let mut command = cfg_space::Command::from_bits(val as u16); + if command.into_bits() & !SUPPORTED_COMMAND_BITS != 0 { + tracelimit::warn_ratelimited!(offset, val, "setting invalid command bits"); + // still do our best + command = + cfg_space::Command::from_bits(command.into_bits() & SUPPORTED_COMMAND_BITS); + }; + + if self.state.command.intx_disable() != command.intx_disable() { + self.update_intx_disable(command.intx_disable()) + } + + if self.state.command.mmio_enabled() != command.mmio_enabled() { + self.update_mmio_enabled(command.mmio_enabled()) + } + + self.state.command = command; + } + // Capabilities space - handled by common emulator + _ if (0x40..0x100).contains(&offset) => { + return self.write_capabilities(offset, val); + } + // Extended capabilities space - handled by common emulator + _ if (0x100..0x1000).contains(&offset) => { + return self.write_extended_capabilities(offset, val); + } + // Check if this is a BAR write (Type 0: 0x10-0x27, Type 1: 0x10-0x17) + _ if self.is_bar_offset(offset) => { + return self.write_bar(offset, val); + } + // Unhandled access - not part of common header, caller should handle + _ => { + return CommonHeaderResult::Unhandled; + } + } + + // Handled access + CommonHeaderResult::Handled + } + + /// Helper for reading BAR registers + fn read_bar(&self, offset: u16, value: &mut u32) -> CommonHeaderResult { + if !self.is_bar_offset(offset) { + return CommonHeaderResult::Unhandled; + } + + let bar_index = self.get_bar_index(offset); + if bar_index < N { + *value = self.state.base_addresses[bar_index]; + } else { + *value = 0; + } + CommonHeaderResult::Handled + } + + /// Helper for writing BAR registers + fn write_bar(&mut self, offset: u16, val: u32) -> CommonHeaderResult { + if !self.is_bar_offset(offset) { + return CommonHeaderResult::Unhandled; + } + + // Handle BAR writes - only allow when MMIO is disabled + if !self.state.command.mmio_enabled() { + let bar_index = self.get_bar_index(offset); + if bar_index < N { + let mut bar_value = val & self.bar_masks[bar_index]; + + // For even-indexed BARs, set the 64-bit type bit if the BAR is configured + if bar_index & 1 == 0 && self.bar_masks[bar_index] != 0 { + bar_value = cfg_space::BarEncodingBits::from_bits(bar_value) + .with_type_64_bit(true) + .into_bits(); + } + + self.state.base_addresses[bar_index] = bar_value; + } + } + CommonHeaderResult::Handled + } + + /// Read from capabilities space. `offset` must be 32-bit aligned and >= 0x40. + fn read_capabilities(&self, offset: u16, value: &mut u32) -> CommonHeaderResult { + if (0x40..0x100).contains(&offset) { + if let Some((cap_index, cap_offset)) = + self.get_capability_index_and_offset(offset - 0x40) + { + *value = self.capabilities[cap_index].read_u32(cap_offset); + if cap_offset == 0 { + let next = if cap_index < self.capabilities.len() - 1 { + offset as u32 + self.capabilities[cap_index].len() as u32 + } else { + 0 + }; + assert!(*value & 0xff00 == 0); + *value |= next << 8; + } + CommonHeaderResult::Handled + } else { + tracelimit::warn_ratelimited!(offset, "unhandled config space read"); + CommonHeaderResult::Failed(IoError::InvalidRegister) + } + } else { + CommonHeaderResult::Failed(IoError::InvalidRegister) + } + } + + /// Write to capabilities space. `offset` must be 32-bit aligned and >= 0x40. + fn write_capabilities(&mut self, offset: u16, val: u32) -> CommonHeaderResult { + if (0x40..0x100).contains(&offset) { + if let Some((cap_index, cap_offset)) = + self.get_capability_index_and_offset(offset - 0x40) + { + self.capabilities[cap_index].write_u32(cap_offset, val); + CommonHeaderResult::Handled + } else { + tracelimit::warn_ratelimited!(offset, value = val, "unhandled config space write"); + CommonHeaderResult::Failed(IoError::InvalidRegister) + } + } else { + CommonHeaderResult::Failed(IoError::InvalidRegister) + } + } + + /// Read from extended capabilities space (0x100-0x1000). `offset` must be 32-bit aligned. + fn read_extended_capabilities(&self, offset: u16, value: &mut u32) -> CommonHeaderResult { + if (0x100..0x1000).contains(&offset) { + if self.is_pcie_device() { + *value = 0xffffffff; + CommonHeaderResult::Handled + } else { + tracelimit::warn_ratelimited!(offset, "unhandled extended config space read"); + CommonHeaderResult::Failed(IoError::InvalidRegister) + } + } else { + CommonHeaderResult::Failed(IoError::InvalidRegister) + } + } + + /// Write to extended capabilities space (0x100-0x1000). `offset` must be 32-bit aligned. + fn write_extended_capabilities(&mut self, offset: u16, val: u32) -> CommonHeaderResult { + if (0x100..0x1000).contains(&offset) { + if self.is_pcie_device() { + // For now, just ignore writes to extended config space + CommonHeaderResult::Handled + } else { + tracelimit::warn_ratelimited!( + offset, + value = val, + "unhandled extended config space write" + ); + CommonHeaderResult::Failed(IoError::InvalidRegister) + } + } else { + CommonHeaderResult::Failed(IoError::InvalidRegister) + } + } + + // ===== Utility and Query Functions ===== + + /// Finds a BAR + offset by address. + pub fn find_bar(&self, address: u64) -> Option<(u8, u16)> { + self.active_bars.find(address) + } + + /// Check if this device is a PCIe device by looking for the PCI Express capability. + pub fn is_pcie_device(&self) -> bool { + self.capabilities + .iter() + .any(|cap| cap.capability_id() == CapabilityId::PCI_EXPRESS) + } + + /// Get capability index and offset for a given offset + fn get_capability_index_and_offset(&self, offset: u16) -> Option<(usize, u16)> { + let mut cap_offset = 0; + for i in 0..self.capabilities.len() { + let cap_size = self.capabilities[i].len() as u16; + if offset < cap_offset + cap_size { + return Some((i, offset - cap_offset)); + } + cap_offset += cap_size; + } + None + } + + /// Check if an offset corresponds to a BAR register + fn is_bar_offset(&self, offset: u16) -> bool { + // Type 0: BAR0-BAR5 (0x10-0x27), Type 1: BAR0-BAR1 (0x10-0x17) + let bar_start = cfg_space::HeaderType00::BAR0.0; + let bar_end = bar_start + (N as u16) * 4; + (bar_start..bar_end).contains(&offset) && offset.is_multiple_of(4) + } + + /// Get the BAR index for a given offset + fn get_bar_index(&self, offset: u16) -> usize { + ((offset - cfg_space::HeaderType00::BAR0.0) / 4) as usize + } + + /// Get BAR masks (for testing only) + #[cfg(test)] + pub fn bar_masks(&self) -> &[u32; N] { + &self.bar_masks + } +} + +#[derive(Debug, Inspect)] +struct ConfigSpaceType0EmulatorState { + /// A read/write register that doesn't matter in virtualized contexts + latency_timer: u8, +} + +impl ConfigSpaceType0EmulatorState { + fn new() -> Self { + Self { latency_timer: 0 } + } +} + +/// Emulator for the standard Type 0 PCI configuration space header. +#[derive(Inspect)] +pub struct ConfigSpaceType0Emulator { + /// The common header emulator that handles shared functionality + #[inspect(flatten)] + common: ConfigSpaceCommonHeaderEmulatorType0, + /// Type 0 specific state state: ConfigSpaceType0EmulatorState, } mod inspect_helpers { use super::*; - pub(crate) fn bars(bars: &[u32; 6]) -> impl Inspect + '_ { + pub(crate) fn bars_generic(bars: &[u32; N]) -> impl Inspect + '_ { inspect::AsHex(inspect::iter_by_index(bars).prefix("bar")) } } @@ -217,76 +771,39 @@ impl DeviceBars { pub fn bar0(mut self, len: u64, memory: BarMemoryKind) -> Self { self.bars[0] = Some((len, memory)); self - } - - /// Set BAR2 - pub fn bar2(mut self, len: u64, memory: BarMemoryKind) -> Self { - self.bars[2] = Some((len, memory)); - self - } - - /// Set BAR4 - pub fn bar4(mut self, len: u64, memory: BarMemoryKind) -> Self { - self.bars[4] = Some((len, memory)); - self - } -} - -impl ConfigSpaceType0Emulator { - /// Create a new [`ConfigSpaceType0Emulator`] - pub fn new( - hardware_ids: HardwareIds, - capabilities: Vec>, - bars: DeviceBars, - ) -> Self { - let mut bar_masks = [0; 6]; - let mut mapped_memory = { - const NONE: Option = None; - [NONE; 6] - }; - for (bar_index, bar) in bars.bars.into_iter().enumerate() { - let (len, mapped) = match bar { - Some(bar) => bar, - None => continue, - }; - // use 64-bit aware BARs - assert!(bar_index < 5); - // Round up regions to a power of 2, as required by PCI (and - // inherently required by the BAR representation). Round up to at - // least one page to avoid various problems in guest OSes. - const MIN_BAR_SIZE: u64 = 4096; - let len = std::cmp::max(len.next_power_of_two(), MIN_BAR_SIZE); - let mask64 = !(len - 1); - bar_masks[bar_index] = cfg_space::BarEncodingBits::from_bits(mask64 as u32) - .with_type_64_bit(true) - .into_bits(); - bar_masks[bar_index + 1] = (mask64 >> 32) as u32; - mapped_memory[bar_index] = Some(mapped); - } - - Self { - bar_masks, - hardware_ids, - multi_function_bit: false, + } - active_bars: Default::default(), + /// Set BAR2 + pub fn bar2(mut self, len: u64, memory: BarMemoryKind) -> Self { + self.bars[2] = Some((len, memory)); + self + } - mapped_memory, - capabilities, - intx_interrupt: None, + /// Set BAR4 + pub fn bar4(mut self, len: u64, memory: BarMemoryKind) -> Self { + self.bars[4] = Some((len, memory)); + self + } +} - state: ConfigSpaceType0EmulatorState { - command: cfg_space::Command::new(), - base_addresses: [0; 6], - interrupt_line: 0, - latency_timer: 0, - }, +impl ConfigSpaceType0Emulator { + /// Create a new [`ConfigSpaceType0Emulator`] + pub fn new( + hardware_ids: HardwareIds, + capabilities: Vec>, + bars: DeviceBars, + ) -> Self { + let common = ConfigSpaceCommonHeaderEmulator::new(hardware_ids, capabilities, bars); + + Self { + common, + state: ConfigSpaceType0EmulatorState::new(), } } /// If the device is multi-function, enable bit 7 in the Header register. pub fn with_multi_function_bit(mut self, bit: bool) -> Self { - self.multi_function_bit = bit; + self.common = self.common.with_multi_function_bit(bit); self } @@ -298,142 +815,50 @@ impl ConfigSpaceType0Emulator { pin: PciInterruptPin, line: LineInterrupt, ) -> Arc { - let intx_interrupt = Arc::new(IntxInterrupt { - pin, - line, - interrupt_disabled: AtomicBool::new(false), - interrupt_status: AtomicBool::new(false), - }); - self.intx_interrupt = Some(intx_interrupt.clone()); - intx_interrupt + self.common.set_interrupt_pin(pin, line) } /// Resets the configuration space state. pub fn reset(&mut self) { + self.common.reset(); self.state = ConfigSpaceType0EmulatorState::new(); - - self.sync_command_register(self.state.command); - - for cap in &mut self.capabilities { - cap.reset(); - } - - if let Some(intx) = &mut self.intx_interrupt { - intx.set_level(false); - } - } - - fn get_capability_index_and_offset(&self, offset: u16) -> Option<(usize, u16)> { - let mut cap_offset = 0; - for i in 0..self.capabilities.len() { - let cap_size = self.capabilities[i].len() as u16; - if offset < cap_offset + cap_size { - return Some((i, offset - cap_offset)); - } - cap_offset += cap_size; - } - None } /// Read from the config space. `offset` must be 32-bit aligned. pub fn read_u32(&self, offset: u16, value: &mut u32) -> IoResult { use cfg_space::HeaderType00; - *value = match HeaderType00(offset) { - HeaderType00::DEVICE_VENDOR => { - (self.hardware_ids.device_id as u32) << 16 | self.hardware_ids.vendor_id as u32 + // First try to handle with common header emulator + match self.common.read_u32(offset, value) { + CommonHeaderResult::Handled => return IoResult::Ok, + CommonHeaderResult::Failed(err) => return IoResult::Err(err), + CommonHeaderResult::Unhandled => { + // Continue with Type 0 specific handling } - HeaderType00::STATUS_COMMAND => { - let mut status = - cfg_space::Status::new().with_capabilities_list(!self.capabilities.is_empty()); - - if let Some(intx_interrupt) = &self.intx_interrupt { - if intx_interrupt.interrupt_status.load(Ordering::SeqCst) { - status.set_interrupt_status(true); - } - } + } - (status.into_bits() as u32) << 16 | self.state.command.into_bits() as u32 - } - HeaderType00::CLASS_REVISION => { - (u8::from(self.hardware_ids.base_class) as u32) << 24 - | (u8::from(self.hardware_ids.sub_class) as u32) << 16 - | (u8::from(self.hardware_ids.prog_if) as u32) << 8 - | self.hardware_ids.revision_id as u32 - } + // Handle Type 0 specific registers + *value = match HeaderType00(offset) { HeaderType00::BIST_HEADER => { let mut v = (self.state.latency_timer as u32) << 8; - if self.multi_function_bit { + if self.common.multi_function_bit() { // enable top-most bit of the header register v |= 0x80 << 16; } v } - HeaderType00::BAR0 - | HeaderType00::BAR1 - | HeaderType00::BAR2 - | HeaderType00::BAR3 - | HeaderType00::BAR4 - | HeaderType00::BAR5 => { - self.state.base_addresses[(offset - HeaderType00::BAR0.0) as usize / 4] - } HeaderType00::CARDBUS_CIS_PTR => 0, HeaderType00::SUBSYSTEM_ID => { - (self.hardware_ids.type0_sub_system_id as u32) << 16 - | self.hardware_ids.type0_sub_vendor_id as u32 + (self.common.hardware_ids().type0_sub_system_id as u32) << 16 + | self.common.hardware_ids().type0_sub_vendor_id as u32 } HeaderType00::EXPANSION_ROM_BASE => 0, - HeaderType00::RESERVED_CAP_PTR => { - if self.capabilities.is_empty() { - 0 - } else { - 0x40 - } - } HeaderType00::RESERVED => 0, HeaderType00::LATENCY_INTERRUPT => { - let interrupt_pin = if let Some(intx_interrupt) = &self.intx_interrupt { - match intx_interrupt.pin { - PciInterruptPin::IntA => 1, - PciInterruptPin::IntB => 2, - PciInterruptPin::IntC => 3, - PciInterruptPin::IntD => 4, - } - } else { - 0 - }; - self.state.interrupt_line as u32 | (interrupt_pin as u32) << 8 - } - // rest of the range is reserved for extended device capabilities - _ if (0x40..0x100).contains(&offset) => { - if let Some((cap_index, cap_offset)) = - self.get_capability_index_and_offset(offset - 0x40) - { - let mut value = self.capabilities[cap_index].read_u32(cap_offset); - if cap_offset == 0 { - let next = if cap_index < self.capabilities.len() - 1 { - offset as u32 + self.capabilities[cap_index].len() as u32 - } else { - 0 - }; - assert!(value & 0xff00 == 0); - value |= next << 8; - } - value - } else { - tracelimit::warn_ratelimited!(offset, "unhandled config space read"); - return IoResult::Err(IoError::InvalidRegister); - } - } - _ if (0x100..0x1000).contains(&offset) => { - // TODO: properly support extended pci express configuration space - if offset == 0x100 { - tracelimit::warn_ratelimited!(offset, "unexpected pci express probe"); - 0x000ffff - } else { - tracelimit::warn_ratelimited!(offset, "unhandled extended config space read"); - return IoResult::Err(IoError::InvalidRegister); - } + // Bits 7-0: Interrupt Line, Bits 15-8: Interrupt Pin, Bits 31-16: Latency Timer + (self.state.latency_timer as u32) << 16 + | (self.common.interrupt_pin() as u32) << 8 + | self.common.interrupt_line() as u32 } _ => { tracelimit::warn_ratelimited!(offset, "unexpected config space read"); @@ -444,119 +869,34 @@ impl ConfigSpaceType0Emulator { IoResult::Ok } - fn update_intx_disable(&mut self, command: cfg_space::Command) { - if let Some(intx_interrupt) = &self.intx_interrupt { - intx_interrupt.set_disabled(command.intx_disable()) - } - } - - fn update_mmio_enabled(&mut self, command: cfg_space::Command) { - if command.mmio_enabled() { - self.active_bars = BarMappings::parse(&self.state.base_addresses, &self.bar_masks); - for (bar, mapping) in self.mapped_memory.iter_mut().enumerate() { - if let Some(mapping) = mapping { - let base = self.active_bars.get(bar as u8).expect("bar exists"); - match mapping.map_to_guest(base) { - Ok(_) => {} - Err(err) => { - tracelimit::error_ratelimited!( - error = &err as &dyn std::error::Error, - bar, - base, - "failed to map bar", - ) - } - } - } - } - } else { - self.active_bars = Default::default(); - for mapping in self.mapped_memory.iter_mut().flatten() { - mapping.unmap_from_guest(); - } - } - } - - fn sync_command_register(&mut self, command: cfg_space::Command) { - self.update_intx_disable(command); - self.update_mmio_enabled(command); - } - /// Write to the config space. `offset` must be 32-bit aligned. pub fn write_u32(&mut self, offset: u16, val: u32) -> IoResult { use cfg_space::HeaderType00; - match HeaderType00(offset) { - HeaderType00::STATUS_COMMAND => { - let mut command = cfg_space::Command::from_bits(val as u16); - if command.into_bits() & !SUPPORTED_COMMAND_BITS != 0 { - tracelimit::warn_ratelimited!(offset, val, "setting invalid command bits"); - // still do our best - command = - cfg_space::Command::from_bits(command.into_bits() & SUPPORTED_COMMAND_BITS); - }; - - if self.state.command.intx_disable() != command.intx_disable() { - self.update_intx_disable(command) - } - - if self.state.command.mmio_enabled() != command.mmio_enabled() { - self.update_mmio_enabled(command) - } - - self.state.command = command; + // First try to handle with common header emulator + match self.common.write_u32(offset, val) { + CommonHeaderResult::Handled => return IoResult::Ok, + CommonHeaderResult::Failed(err) => return IoResult::Err(err), + CommonHeaderResult::Unhandled => { + // Continue with Type 0 specific handling } + } + + // Handle Type 0 specific registers + match HeaderType00(offset) { HeaderType00::BIST_HEADER => { - // allow writes to the latency timer - let timer_val = (val >> 8) as u8; - self.state.latency_timer = timer_val; - } - HeaderType00::BAR0 - | HeaderType00::BAR1 - | HeaderType00::BAR2 - | HeaderType00::BAR3 - | HeaderType00::BAR4 - | HeaderType00::BAR5 => { - if !self.state.command.mmio_enabled() { - let bar_index = (offset - HeaderType00::BAR0.0) as usize / 4; - let mut bar_value = val & self.bar_masks[bar_index]; - if bar_index & 1 == 0 && self.bar_masks[bar_index] != 0 { - bar_value = cfg_space::BarEncodingBits::from_bits(bar_value) - .with_type_64_bit(true) - .into_bits(); - } - self.state.base_addresses[bar_index] = bar_value; - } + // BIST_HEADER - Type 0 specific handling + // For now, just ignore these writes (header type is read-only) } HeaderType00::LATENCY_INTERRUPT => { - self.state.interrupt_line = ((val & 0xff00) >> 8) as u8; + // Bits 7-0: Interrupt Line (read/write) + // Bits 15-8: Interrupt Pin (read-only, ignore writes) + // Bits 31-16: Latency Timer (read/write) + self.common.set_interrupt_line((val & 0xff) as u8); + self.state.latency_timer = (val >> 16) as u8; } // all other base regs are noops _ if offset < 0x40 && offset.is_multiple_of(4) => (), - // rest of the range is reserved for extended device capabilities - _ if (0x40..0x100).contains(&offset) => { - if let Some((cap_index, cap_offset)) = - self.get_capability_index_and_offset(offset - 0x40) - { - self.capabilities[cap_index].write_u32(cap_offset, val); - } else { - tracelimit::warn_ratelimited!( - offset, - value = val, - "unhandled config space write" - ); - return IoResult::Err(IoError::InvalidRegister); - } - } - _ if (0x100..0x1000).contains(&offset) => { - // TODO: properly support extended pci express configuration space - tracelimit::warn_ratelimited!( - offset, - value = val, - "unhandled extended config space write" - ); - return IoResult::Err(IoError::InvalidRegister); - } _ => { tracelimit::warn_ratelimited!(offset, value = val, "unexpected config space write"); return IoResult::Err(IoError::InvalidRegister); @@ -568,14 +908,34 @@ impl ConfigSpaceType0Emulator { /// Finds a BAR + offset by address. pub fn find_bar(&self, address: u64) -> Option<(u8, u16)> { - self.active_bars.find(address) + self.common.find_bar(address) + } + + /// Checks if this device is a PCIe device by looking for the PCI Express capability. + pub fn is_pcie_device(&self) -> bool { + self.common.is_pcie_device() + } + + /// Set the presence detect state for a hotplug-capable slot. + /// This method finds the PCIe Express capability and calls its set_presence_detect_state method. + /// If the PCIe Express capability is not found, the call is silently ignored. + /// + /// # Arguments + /// * `present` - true if a device is present in the slot, false if the slot is empty + pub fn set_presence_detect_state(&mut self, present: bool) { + for capability in self.common.capabilities_mut() { + if let Some(pcie_cap) = capability.as_pci_express_mut() { + pcie_cap.set_presence_detect_state(present); + return; + } + } + + // PCIe Express capability not found - silently ignore } } #[derive(Debug, Inspect)] struct ConfigSpaceType1EmulatorState { - /// The command register - command: cfg_space::Command, /// The subordinate bus number register. Software programs /// this register with the highest bus number below the bridge. subordinate_bus_number: u8, @@ -614,12 +974,14 @@ struct ConfigSpaceType1EmulatorState { /// with the upper 32 bits of the base address of the prefetchable MMIO window /// assigned to the hierarchy under the bridge. prefetch_limit_upper: u32, + /// The bridge control register. Contains various control bits for bridge behavior + /// such as secondary bus reset, VGA enable, etc. + bridge_control: u16, } impl ConfigSpaceType1EmulatorState { fn new() -> Self { Self { - command: cfg_space::Command::new(), subordinate_bus_number: 0, secondary_bus_number: 0, primary_bus_number: 0, @@ -629,47 +991,42 @@ impl ConfigSpaceType1EmulatorState { prefetch_limit: 0, prefetch_base_upper: 0, prefetch_limit_upper: 0, + bridge_control: 0, } } } /// Emulator for the standard Type 1 PCI configuration space header. -// -// TODO: Figure out how to split this up and share the handling of common -// registers (hardware IDs, command, status, etc.) with the type 0 emulator. -// TODO: Support type 1 BARs (only two) #[derive(Inspect)] pub struct ConfigSpaceType1Emulator { - hardware_ids: HardwareIds, - #[inspect(with = "|x| inspect::iter_by_key(x.iter().map(|cap| (cap.label(), cap)))")] - capabilities: Vec>, - multi_function_bit: bool, + /// The common header emulator that handles shared functionality + #[inspect(flatten)] + common: ConfigSpaceCommonHeaderEmulatorType1, + /// Type 1 specific state state: ConfigSpaceType1EmulatorState, } impl ConfigSpaceType1Emulator { /// Create a new [`ConfigSpaceType1Emulator`] pub fn new(hardware_ids: HardwareIds, capabilities: Vec>) -> Self { + let common = + ConfigSpaceCommonHeaderEmulator::new(hardware_ids, capabilities, DeviceBars::new()); + Self { - hardware_ids, - capabilities, - multi_function_bit: false, + common, state: ConfigSpaceType1EmulatorState::new(), } } /// Resets the configuration space state. pub fn reset(&mut self) { + self.common.reset(); self.state = ConfigSpaceType1EmulatorState::new(); - - for cap in &mut self.capabilities { - cap.reset(); - } } /// Set the multi-function bit for this device. pub fn with_multi_function_bit(mut self, multi_function: bool) -> Self { - self.multi_function_bit = multi_function; + self.common = self.common.with_multi_function_bit(multi_function); self } @@ -695,7 +1052,7 @@ impl ConfigSpaceType1Emulator { pub fn assigned_memory_range(&self) -> Option> { let (base_addr, limit_addr) = self.decode_memory_range(self.state.memory_base, self.state.memory_limit); - if self.state.command.mmio_enabled() && base_addr <= limit_addr { + if self.common.command().mmio_enabled() && base_addr <= limit_addr { Some(base_addr..=limit_addr) } else { None @@ -709,55 +1066,36 @@ impl ConfigSpaceType1Emulator { self.decode_memory_range(self.state.prefetch_base, self.state.prefetch_limit); let base_addr = (self.state.prefetch_base_upper as u64) << 32 | base_low as u64; let limit_addr = (self.state.prefetch_limit_upper as u64) << 32 | limit_low as u64; - if self.state.command.mmio_enabled() && base_addr <= limit_addr { + if self.common.command().mmio_enabled() && base_addr <= limit_addr { Some(base_addr..=limit_addr) } else { None } } - fn get_capability_index_and_offset(&self, offset: u16) -> Option<(usize, u16)> { - let mut cap_offset = 0; - for i in 0..self.capabilities.len() { - let cap_size = self.capabilities[i].len() as u16; - if offset < cap_offset + cap_size { - return Some((i, offset - cap_offset)); - } - cap_offset += cap_size; - } - None - } - /// Read from the config space. `offset` must be 32-bit aligned. pub fn read_u32(&self, offset: u16, value: &mut u32) -> IoResult { use cfg_space::HeaderType01; - *value = match HeaderType01(offset) { - HeaderType01::DEVICE_VENDOR => { - (self.hardware_ids.device_id as u32) << 16 | self.hardware_ids.vendor_id as u32 + // First try to handle with common header emulator + match self.common.read_u32(offset, value) { + CommonHeaderResult::Handled => return IoResult::Ok, + CommonHeaderResult::Failed(err) => return IoResult::Err(err), + CommonHeaderResult::Unhandled => { + // Continue with Type 1 specific handling } - HeaderType01::STATUS_COMMAND => { - let status = - cfg_space::Status::new().with_capabilities_list(!self.capabilities.is_empty()); + } - (status.into_bits() as u32) << 16 | self.state.command.into_bits() as u32 - } - HeaderType01::CLASS_REVISION => { - (u8::from(self.hardware_ids.base_class) as u32) << 24 - | (u8::from(self.hardware_ids.sub_class) as u32) << 16 - | (u8::from(self.hardware_ids.prog_if) as u32) << 8 - | self.hardware_ids.revision_id as u32 - } + // Handle Type 1 specific registers + *value = match HeaderType01(offset) { HeaderType01::BIST_HEADER => { // Header type 01 with optional multi-function bit - if self.multi_function_bit { + if self.common.multi_function_bit() { 0x00810000 // Header type 01 with multi-function bit (bit 23) } else { 0x00010000 // Header type 01 without multi-function bit } } - HeaderType01::BAR0 => 0, - HeaderType01::BAR1 => 0, HeaderType01::LATENCY_BUS_NUMBERS => { (self.state.subordinate_bus_number as u32) << 16 | (self.state.secondary_bus_number as u32) << 8 @@ -776,45 +1114,11 @@ impl ConfigSpaceType1Emulator { HeaderType01::PREFETCH_BASE_UPPER => self.state.prefetch_base_upper, HeaderType01::PREFETCH_LIMIT_UPPER => self.state.prefetch_limit_upper, HeaderType01::IO_RANGE_UPPER => 0, - HeaderType01::RESERVED_CAP_PTR => { - if self.capabilities.is_empty() { - 0 - } else { - 0x40 - } - } HeaderType01::EXPANSION_ROM_BASE => 0, - HeaderType01::BRDIGE_CTRL_INTERRUPT => 0, - // rest of the range is reserved for device capabilities - _ if (0x40..0x100).contains(&offset) => { - if let Some((cap_index, cap_offset)) = - self.get_capability_index_and_offset(offset - 0x40) - { - let mut value = self.capabilities[cap_index].read_u32(cap_offset); - if cap_offset == 0 { - let next = if cap_index < self.capabilities.len() - 1 { - offset as u32 + self.capabilities[cap_index].len() as u32 - } else { - 0 - }; - assert!(value & 0xff00 == 0); - value |= next << 8; - } - value - } else { - tracelimit::warn_ratelimited!(offset, "unhandled config space read"); - return IoResult::Err(IoError::InvalidRegister); - } - } - _ if (0x100..0x1000).contains(&offset) => { - // TODO: properly support extended pci express configuration space - if offset == 0x100 { - tracelimit::warn_ratelimited!(offset, "unexpected pci express probe"); - 0x000ffff - } else { - tracelimit::warn_ratelimited!(offset, "unhandled extended config space read"); - return IoResult::Err(IoError::InvalidRegister); - } + HeaderType01::BRDIGE_CTRL_INTERRUPT => { + // Read interrupt line from common header and bridge control from state + // Bits 7-0: Interrupt Line, Bits 15-8: Interrupt Pin (0), Bits 31-16: Bridge Control + (self.state.bridge_control as u32) << 16 | self.common.interrupt_line() as u32 } _ => { tracelimit::warn_ratelimited!(offset, "unexpected config space read"); @@ -829,20 +1133,20 @@ impl ConfigSpaceType1Emulator { pub fn write_u32(&mut self, offset: u16, val: u32) -> IoResult { use cfg_space::HeaderType01; - match HeaderType01(offset) { - HeaderType01::STATUS_COMMAND => { - let mut command = cfg_space::Command::from_bits(val as u16); - if command.into_bits() & !SUPPORTED_COMMAND_BITS != 0 { - tracelimit::warn_ratelimited!(offset, val, "setting invalid command bits"); - // still do our best - command = - cfg_space::Command::from_bits(command.into_bits() & SUPPORTED_COMMAND_BITS); - }; - - // TODO: when the memory space enable bit is written, sanity check the programmed - // memory and prefetch ranges... + // First try to handle with common header emulator + match self.common.write_u32(offset, val) { + CommonHeaderResult::Handled => return IoResult::Ok, + CommonHeaderResult::Failed(err) => return IoResult::Err(err), + CommonHeaderResult::Unhandled => { + // Continue with Type 1 specific handling + } + } - self.state.command = command; + // Handle Type 1 specific registers + match HeaderType01(offset) { + HeaderType01::BIST_HEADER => { + // BIST_HEADER - Type 1 specific handling + // For now, just ignore these writes (latency timer would go here if supported) } HeaderType01::LATENCY_BUS_NUMBERS => { self.state.subordinate_bus_number = (val >> 16) as u8; @@ -863,39 +1167,46 @@ impl ConfigSpaceType1Emulator { HeaderType01::PREFETCH_LIMIT_UPPER => { self.state.prefetch_limit_upper = val; } + HeaderType01::BRDIGE_CTRL_INTERRUPT => { + // Delegate interrupt line writes to common header and store bridge control + // Bits 7-0: Interrupt Line, Bits 15-8: Interrupt Pin (ignored), Bits 31-16: Bridge Control + self.common.set_interrupt_line((val & 0xff) as u8); + self.state.bridge_control = (val >> 16) as u16; + } // all other base regs are noops _ if offset < 0x40 && offset.is_multiple_of(4) => (), - // rest of the range is reserved for extended device capabilities - _ if (0x40..0x100).contains(&offset) => { - if let Some((cap_index, cap_offset)) = - self.get_capability_index_and_offset(offset - 0x40) - { - self.capabilities[cap_index].write_u32(cap_offset, val); - } else { - tracelimit::warn_ratelimited!( - offset, - value = val, - "unhandled config space write" - ); - return IoResult::Err(IoError::InvalidRegister); - } - } - _ if (0x100..0x1000).contains(&offset) => { - // TODO: properly support extended pci express configuration space - tracelimit::warn_ratelimited!( - offset, - value = val, - "unhandled extended config space write" - ); - return IoResult::Err(IoError::InvalidRegister); - } _ => { tracelimit::warn_ratelimited!(offset, value = val, "unexpected config space write"); return IoResult::Err(IoError::InvalidRegister); } } - - IoResult::Ok + + IoResult::Ok + } + + /// Checks if this device is a PCIe device by looking for the PCI Express capability. + pub fn is_pcie_device(&self) -> bool { + self.common.is_pcie_device() + } + + /// Set the presence detect state for the slot. + /// This method finds the PCIe Express capability and calls its set_presence_detect_state method. + /// If the PCIe Express capability is not found, the call is silently ignored. + /// + /// # Arguments + /// * `present` - true if a device is present in the slot, false if the slot is empty + pub fn set_presence_detect_state(&mut self, present: bool) { + // Find the PCIe Express capability + for cap in self.common.capabilities_mut() { + if cap.capability_id() == CapabilityId::PCI_EXPRESS { + // Downcast to PciExpressCapability and call set_presence_detect_state + if let Some(pcie_cap) = cap.as_pci_express_mut() { + pcie_cap.set_presence_detect_state(present); + return; + } + } + } + // If no PCIe Express capability is found, silently ignore the call } } @@ -935,22 +1246,42 @@ mod save_restore { InvalidCap(String), } - impl SaveRestore for ConfigSpaceType0Emulator { + impl SaveRestore for ConfigSpaceCommonHeaderEmulator { type SavedState = state::SavedState; fn save(&mut self) -> Result { - let ConfigSpaceType0EmulatorState { + tracing::info!( + "ConfigSpaceCommonHeaderEmulator<{}>: starting save operation", + N + ); + + let ConfigSpaceCommonHeaderEmulatorState { command, base_addresses, interrupt_line, - latency_timer, } = self.state; + // Convert to 6-element array, padding with zeros if needed + let mut saved_base_addresses = [0u32; 6]; + for (i, &addr) in base_addresses.iter().enumerate() { + if i < 6 { + saved_base_addresses[i] = addr; + } + } + + tracing::info!( + "ConfigSpaceCommonHeaderEmulator<{}>: saving state - command={:#x}, interrupt_line={}, base_addresses={:?}", + N, + command.into_bits(), + interrupt_line, + saved_base_addresses + ); + let saved_state = state::SavedState { command: command.into_bits(), - base_addresses, + base_addresses: saved_base_addresses, interrupt_line, - latency_timer, + latency_timer: 0, // Not used in common header, always 0 capabilities: self .capabilities .iter_mut() @@ -961,34 +1292,79 @@ mod save_restore { .collect::>()?, }; + tracing::info!( + "ConfigSpaceCommonHeaderEmulator<{}>: save operation completed successfully", + N + ); Ok(saved_state) } fn restore(&mut self, state: Self::SavedState) -> Result<(), RestoreError> { + tracing::info!( + "ConfigSpaceCommonHeaderEmulator<{}>: starting restore operation", + N + ); + let state::SavedState { command, base_addresses, interrupt_line, - latency_timer, + latency_timer: _, // Ignore latency_timer field capabilities, } = state; - self.state = ConfigSpaceType0EmulatorState { + tracing::info!( + "ConfigSpaceCommonHeaderEmulator<{}>: restoring state - command={:#x}, interrupt_line={}, base_addresses={:?}", + N, + command, + interrupt_line, + base_addresses + ); + + // Convert from 6-element array, taking only what we need + let mut restored_base_addresses = { + const ZERO: u32 = 0; + [ZERO; N] + }; + for (i, &addr) in base_addresses.iter().enumerate() { + if i < N { + restored_base_addresses[i] = addr; + } + } + + self.state = ConfigSpaceCommonHeaderEmulatorState { command: cfg_space::Command::from_bits(command), - base_addresses, + base_addresses: restored_base_addresses, interrupt_line, - latency_timer, }; if command & !SUPPORTED_COMMAND_BITS != 0 { + tracing::warn!( + "ConfigSpaceCommonHeaderEmulator<{}>: invalid command bits found: {:#x}", + N, + command + ); return Err(RestoreError::InvalidSavedState( ConfigSpaceRestoreError::InvalidConfigBits.into(), )); } + tracing::info!( + "ConfigSpaceCommonHeaderEmulator<{}>: syncing command register", + N + ); self.sync_command_register(self.state.command); + + tracing::info!( + "ConfigSpaceCommonHeaderEmulator<{}>: restoring {} capabilities", + N, + capabilities.len() + ); for (id, entry) in capabilities { - tracing::debug!(save_id = id.as_str(), "restoring pci capability"); + tracing::debug!( + save_id = id.as_str(), + "restoring pci common header capability" + ); // yes, yes, this is O(n^2), but devices never have more than a // handful of caps, so it's totally fine. @@ -1002,12 +1378,182 @@ mod save_restore { } if !restored { + tracing::error!( + "ConfigSpaceCommonHeaderEmulator<{}>: failed to find capability: {}", + N, + id + ); return Err(RestoreError::InvalidSavedState( ConfigSpaceRestoreError::InvalidCap(id).into(), )); } } + tracing::info!( + "ConfigSpaceCommonHeaderEmulator<{}>: restore operation completed successfully", + N + ); + Ok(()) + } + } + + mod type0_state { + use super::state; + use mesh::payload::Protobuf; + use vmcore::save_restore::SavedStateRoot; + + #[derive(Protobuf, SavedStateRoot)] + #[mesh(package = "pci.cfg_space_emu")] + pub struct SavedType0State { + #[mesh(1)] + pub latency_timer: u8, + #[mesh(2)] + pub common_header: state::SavedState, + } + } + + impl SaveRestore for ConfigSpaceType0Emulator { + type SavedState = type0_state::SavedType0State; + + fn save(&mut self) -> Result { + tracing::info!("ConfigSpaceType0Emulator: starting save operation"); + + let ConfigSpaceType0EmulatorState { latency_timer } = self.state; + + tracing::info!( + "ConfigSpaceType0Emulator: saving latency_timer={}", + latency_timer + ); + + let saved_state = type0_state::SavedType0State { + latency_timer, + common_header: self.common.save()?, + }; + + tracing::info!("ConfigSpaceType0Emulator: save operation completed successfully"); + Ok(saved_state) + } + + fn restore(&mut self, state: Self::SavedState) -> Result<(), RestoreError> { + tracing::info!("ConfigSpaceType0Emulator: starting restore operation"); + + let type0_state::SavedType0State { + latency_timer, + common_header, + } = state; + + tracing::info!( + "ConfigSpaceType0Emulator: restoring latency_timer={}", + latency_timer + ); + + self.state = ConfigSpaceType0EmulatorState { latency_timer }; + + tracing::info!("ConfigSpaceType0Emulator: delegating to common header restore"); + self.common.restore(common_header)?; + + tracing::info!("ConfigSpaceType0Emulator: restore operation completed successfully"); + Ok(()) + } + } + + mod type1_state { + use super::state; + use mesh::payload::Protobuf; + use vmcore::save_restore::SavedStateRoot; + + #[derive(Protobuf, SavedStateRoot)] + #[mesh(package = "pci.cfg_space_emu")] + pub struct SavedType1State { + #[mesh(1)] + pub subordinate_bus_number: u8, + #[mesh(2)] + pub secondary_bus_number: u8, + #[mesh(3)] + pub primary_bus_number: u8, + #[mesh(4)] + pub memory_base: u16, + #[mesh(5)] + pub memory_limit: u16, + #[mesh(6)] + pub prefetch_base: u16, + #[mesh(7)] + pub prefetch_limit: u16, + #[mesh(8)] + pub prefetch_base_upper: u32, + #[mesh(9)] + pub prefetch_limit_upper: u32, + #[mesh(10)] + pub bridge_control: u16, + #[mesh(11)] + pub common_header: state::SavedState, + } + } + + impl SaveRestore for ConfigSpaceType1Emulator { + type SavedState = type1_state::SavedType1State; + + fn save(&mut self) -> Result { + let ConfigSpaceType1EmulatorState { + subordinate_bus_number, + secondary_bus_number, + primary_bus_number, + memory_base, + memory_limit, + prefetch_base, + prefetch_limit, + prefetch_base_upper, + prefetch_limit_upper, + bridge_control, + } = self.state; + + let saved_state = type1_state::SavedType1State { + subordinate_bus_number, + secondary_bus_number, + primary_bus_number, + memory_base, + memory_limit, + prefetch_base, + prefetch_limit, + prefetch_base_upper, + prefetch_limit_upper, + bridge_control, + common_header: self.common.save()?, + }; + + Ok(saved_state) + } + + fn restore(&mut self, state: Self::SavedState) -> Result<(), RestoreError> { + let type1_state::SavedType1State { + subordinate_bus_number, + secondary_bus_number, + primary_bus_number, + memory_base, + memory_limit, + prefetch_base, + prefetch_limit, + prefetch_base_upper, + prefetch_limit_upper, + bridge_control, + common_header, + } = state; + + self.state = ConfigSpaceType1EmulatorState { + subordinate_bus_number, + secondary_bus_number, + primary_bus_number, + memory_base, + memory_limit, + prefetch_base, + prefetch_limit, + prefetch_base_upper, + prefetch_limit_upper, + bridge_control, + }; + + self.common.restore(common_header)?; + Ok(()) } } @@ -1016,7 +1562,9 @@ mod save_restore { #[cfg(test)] mod tests { use super::*; + use crate::capabilities::pci_express::PciExpressCapability; use crate::capabilities::read_only::ReadOnlyCapability; + use crate::spec::caps::pci_express::DevicePortType; use crate::spec::hwid::ClassCode; use crate::spec::hwid::ProgrammingInterface; use crate::spec::hwid::Subclass; @@ -1186,4 +1734,525 @@ mod tests { emu.write_u32(0x4, MMIO_DISABLED).unwrap(); assert!(emu.assigned_prefetch_range().is_none()); } + + #[test] + fn test_type1_is_pcie_device() { + // Test Type 1 device without PCIe capability + let emu = create_type1_emulator(vec![Box::new(ReadOnlyCapability::new("foo", 0))]); + assert!(!emu.is_pcie_device()); + + // Test Type 1 device with PCIe capability + let emu = create_type1_emulator(vec![Box::new(PciExpressCapability::new( + DevicePortType::RootPort, + None, + ))]); + assert!(emu.is_pcie_device()); + + // Test Type 1 device with multiple capabilities including PCIe + let emu = create_type1_emulator(vec![ + Box::new(ReadOnlyCapability::new("foo", 0)), + Box::new(PciExpressCapability::new(DevicePortType::Endpoint, None)), + Box::new(ReadOnlyCapability::new("bar", 0)), + ]); + assert!(emu.is_pcie_device()); + } + + #[test] + fn test_type0_is_pcie_device() { + // Test Type 0 device without PCIe capability + let emu = ConfigSpaceType0Emulator::new( + HardwareIds { + vendor_id: 0x1111, + device_id: 0x2222, + revision_id: 1, + prog_if: ProgrammingInterface::NONE, + sub_class: Subclass::NONE, + base_class: ClassCode::UNCLASSIFIED, + type0_sub_vendor_id: 0, + type0_sub_system_id: 0, + }, + vec![Box::new(ReadOnlyCapability::new("foo", 0))], + DeviceBars::new(), + ); + assert!(!emu.is_pcie_device()); + + // Test Type 0 device with PCIe capability + let emu = ConfigSpaceType0Emulator::new( + HardwareIds { + vendor_id: 0x1111, + device_id: 0x2222, + revision_id: 1, + prog_if: ProgrammingInterface::NONE, + sub_class: Subclass::NONE, + base_class: ClassCode::UNCLASSIFIED, + type0_sub_vendor_id: 0, + type0_sub_system_id: 0, + }, + vec![Box::new(PciExpressCapability::new( + DevicePortType::Endpoint, + None, + ))], + DeviceBars::new(), + ); + assert!(emu.is_pcie_device()); + + // Test Type 0 device with multiple capabilities including PCIe + let emu = ConfigSpaceType0Emulator::new( + HardwareIds { + vendor_id: 0x1111, + device_id: 0x2222, + revision_id: 1, + prog_if: ProgrammingInterface::NONE, + sub_class: Subclass::NONE, + base_class: ClassCode::UNCLASSIFIED, + type0_sub_vendor_id: 0, + type0_sub_system_id: 0, + }, + vec![ + Box::new(ReadOnlyCapability::new("foo", 0)), + Box::new(PciExpressCapability::new(DevicePortType::Endpoint, None)), + Box::new(ReadOnlyCapability::new("bar", 0)), + ], + DeviceBars::new(), + ); + assert!(emu.is_pcie_device()); + + // Test Type 0 device with no capabilities + let emu = ConfigSpaceType0Emulator::new( + HardwareIds { + vendor_id: 0x1111, + device_id: 0x2222, + revision_id: 1, + prog_if: ProgrammingInterface::NONE, + sub_class: Subclass::NONE, + base_class: ClassCode::UNCLASSIFIED, + type0_sub_vendor_id: 0, + type0_sub_system_id: 0, + }, + vec![], + DeviceBars::new(), + ); + assert!(!emu.is_pcie_device()); + } + + #[test] + fn test_capability_ids() { + // Test that capabilities return the correct capability IDs + let pcie_cap = PciExpressCapability::new(DevicePortType::Endpoint, None); + assert_eq!(pcie_cap.capability_id(), CapabilityId::PCI_EXPRESS); + + let read_only_cap = ReadOnlyCapability::new("test", 0u32); + assert_eq!(read_only_cap.capability_id(), CapabilityId::VENDOR_SPECIFIC); + } + + #[test] + fn test_common_header_emulator_type0() { + // Test the common header emulator with Type 0 configuration (6 BARs) + let hardware_ids = HardwareIds { + vendor_id: 0x1111, + device_id: 0x2222, + revision_id: 1, + prog_if: ProgrammingInterface::NONE, + sub_class: Subclass::NONE, + base_class: ClassCode::UNCLASSIFIED, + type0_sub_vendor_id: 0, + type0_sub_system_id: 0, + }; + + let bars = DeviceBars::new().bar0(4096, BarMemoryKind::Dummy); + + let common_emu: ConfigSpaceCommonHeaderEmulatorType0 = + ConfigSpaceCommonHeaderEmulator::new(hardware_ids, vec![], bars); + + assert_eq!(common_emu.hardware_ids().vendor_id, 0x1111); + assert_eq!(common_emu.hardware_ids().device_id, 0x2222); + assert!(!common_emu.multi_function_bit()); + assert!(!common_emu.is_pcie_device()); + assert_ne!(common_emu.bar_masks()[0], 0); // Should have a mask for BAR0 + } + + #[test] + fn test_common_header_emulator_type1() { + // Test the common header emulator with Type 1 configuration (2 BARs) + let hardware_ids = HardwareIds { + vendor_id: 0x3333, + device_id: 0x4444, + revision_id: 1, + prog_if: ProgrammingInterface::NONE, + sub_class: Subclass::BRIDGE_PCI_TO_PCI, + base_class: ClassCode::BRIDGE, + type0_sub_vendor_id: 0, + type0_sub_system_id: 0, + }; + + let bars = DeviceBars::new().bar0(4096, BarMemoryKind::Dummy); + + let mut common_emu: ConfigSpaceCommonHeaderEmulatorType1 = + ConfigSpaceCommonHeaderEmulator::new( + hardware_ids, + vec![Box::new(PciExpressCapability::new( + DevicePortType::RootPort, + None, + ))], + bars, + ) + .with_multi_function_bit(true); + + assert_eq!(common_emu.hardware_ids().vendor_id, 0x3333); + assert_eq!(common_emu.hardware_ids().device_id, 0x4444); + assert!(common_emu.multi_function_bit()); + assert!(common_emu.is_pcie_device()); + assert_ne!(common_emu.bar_masks()[0], 0); // Should have a mask for BAR0 + assert_eq!(common_emu.bar_masks().len(), 2); + + // Test reset functionality + common_emu.reset(); + assert_eq!(common_emu.capabilities().len(), 1); // capabilities should still be there + } + + #[test] + fn test_common_header_emulator_no_bars() { + // Test the common header emulator with no BARs configured + let hardware_ids = HardwareIds { + vendor_id: 0x5555, + device_id: 0x6666, + revision_id: 1, + prog_if: ProgrammingInterface::NONE, + sub_class: Subclass::NONE, + base_class: ClassCode::UNCLASSIFIED, + type0_sub_vendor_id: 0, + type0_sub_system_id: 0, + }; + + // Create bars with no BARs configured + let bars = DeviceBars::new(); + + let common_emu: ConfigSpaceCommonHeaderEmulatorType0 = + ConfigSpaceCommonHeaderEmulator::new(hardware_ids, vec![], bars); + + assert_eq!(common_emu.hardware_ids().vendor_id, 0x5555); + assert_eq!(common_emu.hardware_ids().device_id, 0x6666); + + // All BAR masks should be 0 when no BARs are configured + for &mask in common_emu.bar_masks() { + assert_eq!(mask, 0); + } + } + + #[test] + fn test_common_header_emulator_type1_ignores_extra_bars() { + // Test that Type 1 emulator ignores BARs beyond index 1 (only supports 2 BARs) + let hardware_ids = HardwareIds { + vendor_id: 0x7777, + device_id: 0x8888, + revision_id: 1, + prog_if: ProgrammingInterface::NONE, + sub_class: Subclass::BRIDGE_PCI_TO_PCI, + base_class: ClassCode::BRIDGE, + type0_sub_vendor_id: 0, + type0_sub_system_id: 0, + }; + + // Configure BARs 0, 2, and 4 - Type 1 should only use BAR0 (and BAR1 as upper 32 bits) + let bars = DeviceBars::new() + .bar0(4096, BarMemoryKind::Dummy) + .bar2(8192, BarMemoryKind::Dummy) + .bar4(16384, BarMemoryKind::Dummy); + + let common_emu: ConfigSpaceCommonHeaderEmulatorType1 = + ConfigSpaceCommonHeaderEmulator::new(hardware_ids, vec![], bars); + + assert_eq!(common_emu.hardware_ids().vendor_id, 0x7777); + assert_eq!(common_emu.hardware_ids().device_id, 0x8888); + + // Should have a mask for BAR0, and BAR1 should be the upper 32 bits (64-bit BAR) + assert_ne!(common_emu.bar_masks()[0], 0); // BAR0 should be configured + assert_ne!(common_emu.bar_masks()[1], 0); // BAR1 should be upper 32 bits of BAR0 + assert_eq!(common_emu.bar_masks().len(), 2); // Type 1 only has 2 BARs + + // BAR2 and higher should be ignored (not accessible in Type 1 with N=2) + // This demonstrates that extra BARs in DeviceBars are properly ignored + } + + #[test] + fn test_common_header_extended_capabilities() { + // Test common header emulator extended capabilities + let mut common_emu_no_pcie = ConfigSpaceCommonHeaderEmulatorType0::new( + HardwareIds { + vendor_id: 0x1111, + device_id: 0x2222, + revision_id: 1, + prog_if: ProgrammingInterface::NONE, + sub_class: Subclass::NONE, + base_class: ClassCode::UNCLASSIFIED, + type0_sub_vendor_id: 0, + type0_sub_system_id: 0, + }, + vec![Box::new(ReadOnlyCapability::new("foo", 0))], + DeviceBars::new(), + ); + assert!(!common_emu_no_pcie.is_pcie_device()); + + let mut common_emu_pcie = ConfigSpaceCommonHeaderEmulatorType0::new( + HardwareIds { + vendor_id: 0x1111, + device_id: 0x2222, + revision_id: 1, + prog_if: ProgrammingInterface::NONE, + sub_class: Subclass::NONE, + base_class: ClassCode::UNCLASSIFIED, + type0_sub_vendor_id: 0, + type0_sub_system_id: 0, + }, + vec![Box::new(PciExpressCapability::new( + DevicePortType::Endpoint, + None, + ))], + DeviceBars::new(), + ); + assert!(common_emu_pcie.is_pcie_device()); + + // Test reading extended capabilities - non-PCIe device should return error + let mut value = 0; + assert!(matches!( + common_emu_no_pcie.read_extended_capabilities(0x100, &mut value), + CommonHeaderResult::Failed(IoError::InvalidRegister) + )); + + // Test reading extended capabilities - PCIe device should return 0xffffffff + let mut value = 0; + assert!(matches!( + common_emu_pcie.read_extended_capabilities(0x100, &mut value), + CommonHeaderResult::Handled + )); + assert_eq!(value, 0xffffffff); + + // Test writing extended capabilities - non-PCIe device should return error + assert!(matches!( + common_emu_no_pcie.write_extended_capabilities(0x100, 0x1234), + CommonHeaderResult::Failed(IoError::InvalidRegister) + )); + + // Test writing extended capabilities - PCIe device should accept writes + assert!(matches!( + common_emu_pcie.write_extended_capabilities(0x100, 0x1234), + CommonHeaderResult::Handled + )); + + // Test invalid offset ranges + let mut value = 0; + assert!(matches!( + common_emu_pcie.read_extended_capabilities(0x99, &mut value), + CommonHeaderResult::Failed(IoError::InvalidRegister) + )); + assert!(matches!( + common_emu_pcie.read_extended_capabilities(0x1000, &mut value), + CommonHeaderResult::Failed(IoError::InvalidRegister) + )); + } + + #[test] + fn test_common_header_emulator_save_restore() { + use vmcore::save_restore::SaveRestore; + + // Test Type 0 common header emulator save/restore + let hardware_ids = HardwareIds { + vendor_id: 0x1111, + device_id: 0x2222, + revision_id: 1, + prog_if: ProgrammingInterface::NONE, + sub_class: Subclass::NONE, + base_class: ClassCode::UNCLASSIFIED, + type0_sub_vendor_id: 0, + type0_sub_system_id: 0, + }; + + let bars = DeviceBars::new().bar0(4096, BarMemoryKind::Dummy); + + let mut common_emu: ConfigSpaceCommonHeaderEmulatorType0 = + ConfigSpaceCommonHeaderEmulator::new(hardware_ids, vec![], bars); + + // Modify some state + let mut test_val = 0u32; + let result = common_emu.write_u32(0x04, 0x0007); // Enable some command bits + assert_eq!(result, CommonHeaderResult::Handled); + let result = common_emu.read_u32(0x04, &mut test_val); + assert_eq!(result, CommonHeaderResult::Handled); + assert_eq!(test_val & 0x0007, 0x0007); + + // Save the state + let saved_state = common_emu.save().expect("save should succeed"); + + // Reset the emulator + common_emu.reset(); + let result = common_emu.read_u32(0x04, &mut test_val); + assert_eq!(result, CommonHeaderResult::Handled); + assert_eq!(test_val & 0x0007, 0x0000); // Should be reset + + // Restore the state + common_emu + .restore(saved_state) + .expect("restore should succeed"); + let result = common_emu.read_u32(0x04, &mut test_val); + assert_eq!(result, CommonHeaderResult::Handled); + assert_eq!(test_val & 0x0007, 0x0007); // Should be restored + + // Test Type 1 common header emulator save/restore + let hardware_ids = HardwareIds { + vendor_id: 0x3333, + device_id: 0x4444, + revision_id: 1, + prog_if: ProgrammingInterface::NONE, + sub_class: Subclass::BRIDGE_PCI_TO_PCI, + base_class: ClassCode::BRIDGE, + type0_sub_vendor_id: 0, + type0_sub_system_id: 0, + }; + + let bars = DeviceBars::new(); // No BARs for Type 1 + + let mut common_emu_type1: ConfigSpaceCommonHeaderEmulatorType1 = + ConfigSpaceCommonHeaderEmulator::new(hardware_ids, vec![], bars); + + // Modify some state + let result = common_emu_type1.write_u32(0x04, 0x0003); // Enable some command bits + assert_eq!(result, CommonHeaderResult::Handled); + let result = common_emu_type1.read_u32(0x04, &mut test_val); + assert_eq!(result, CommonHeaderResult::Handled); + assert_eq!(test_val & 0x0003, 0x0003); + + // Save the state + let saved_state = common_emu_type1.save().expect("save should succeed"); + + // Reset the emulator + common_emu_type1.reset(); + let result = common_emu_type1.read_u32(0x04, &mut test_val); + assert_eq!(result, CommonHeaderResult::Handled); + assert_eq!(test_val & 0x0003, 0x0000); // Should be reset + + // Restore the state + common_emu_type1 + .restore(saved_state) + .expect("restore should succeed"); + let result = common_emu_type1.read_u32(0x04, &mut test_val); + assert_eq!(result, CommonHeaderResult::Handled); + assert_eq!(test_val & 0x0003, 0x0003); // Should be restored + } + + #[test] + fn test_config_space_type1_set_presence_detect_state() { + // Test that ConfigSpaceType1Emulator can set presence detect state + // when it has a PCIe Express capability with hotplug support + + // Create a PCIe Express capability with hotplug support + let pcie_cap = + PciExpressCapability::new(DevicePortType::RootPort, None).with_hotplug_support(1); + + let mut emulator = create_type1_emulator(vec![Box::new(pcie_cap)]); + + // Initially, presence detect state should be 0 + let mut slot_status_val = 0u32; + let result = emulator.read_u32(0x58, &mut slot_status_val); // 0x40 (cap start) + 0x18 (slot control/status) + assert!(matches!(result, IoResult::Ok)); + let initial_presence_detect = (slot_status_val >> 22) & 0x1; // presence_detect_state is bit 6 of slot status + assert_eq!( + initial_presence_detect, 0, + "Initial presence detect state should be 0" + ); + + // Set device as present + emulator.set_presence_detect_state(true); + let result = emulator.read_u32(0x58, &mut slot_status_val); + assert!(matches!(result, IoResult::Ok)); + let present_presence_detect = (slot_status_val >> 22) & 0x1; + assert_eq!( + present_presence_detect, 1, + "Presence detect state should be 1 when device is present" + ); + + // Set device as not present + emulator.set_presence_detect_state(false); + let result = emulator.read_u32(0x58, &mut slot_status_val); + assert!(matches!(result, IoResult::Ok)); + let absent_presence_detect = (slot_status_val >> 22) & 0x1; + assert_eq!( + absent_presence_detect, 0, + "Presence detect state should be 0 when device is not present" + ); + } + + #[test] + fn test_config_space_type1_set_presence_detect_state_without_pcie() { + // Test that ConfigSpaceType1Emulator silently ignores set_presence_detect_state + // when there is no PCIe Express capability + + let mut emulator = create_type1_emulator(vec![]); // No capabilities + + // Should not panic and should be silently ignored + emulator.set_presence_detect_state(true); + emulator.set_presence_detect_state(false); + } + + #[test] + fn test_interrupt_pin_register() { + use vmcore::line_interrupt::LineInterrupt; + + // Test Type 0 device with interrupt pin configured + let mut emu = ConfigSpaceType0Emulator::new( + HardwareIds { + vendor_id: 0x1111, + device_id: 0x2222, + revision_id: 1, + prog_if: ProgrammingInterface::NONE, + sub_class: Subclass::NONE, + base_class: ClassCode::UNCLASSIFIED, + type0_sub_vendor_id: 0, + type0_sub_system_id: 0, + }, + vec![], + DeviceBars::new(), + ); + + // Initially, no interrupt pin should be configured + let mut val = 0u32; + emu.read_u32(0x3C, &mut val).unwrap(); // LATENCY_INTERRUPT register + assert_eq!(val & 0xFF00, 0); // Interrupt pin should be 0 + + // Configure interrupt pin A + let line_interrupt = LineInterrupt::detached(); + emu.set_interrupt_pin(PciInterruptPin::IntA, line_interrupt); + + // Read the register again + emu.read_u32(0x3C, &mut val).unwrap(); + assert_eq!((val >> 8) & 0xFF, 1); // Interrupt pin should be 1 (INTA) + + // Set interrupt line to 0x42 and verify both pin and line are correct + emu.write_u32(0x3C, 0x00110042).unwrap(); // Latency=0x11, pin=ignored, line=0x42 + emu.read_u32(0x3C, &mut val).unwrap(); + assert_eq!(val & 0xFF, 0x42); // Interrupt line should be 0x42 + assert_eq!((val >> 8) & 0xFF, 1); // Interrupt pin should still be 1 (writes ignored) + assert_eq!((val >> 16) & 0xFF, 0x11); // Latency timer should be 0x11 + + // Test with interrupt pin D + let mut emu_d = ConfigSpaceType0Emulator::new( + HardwareIds { + vendor_id: 0x1111, + device_id: 0x2222, + revision_id: 1, + prog_if: ProgrammingInterface::NONE, + sub_class: Subclass::NONE, + base_class: ClassCode::UNCLASSIFIED, + type0_sub_vendor_id: 0, + type0_sub_system_id: 0, + }, + vec![], + DeviceBars::new(), + ); + + let line_interrupt_d = LineInterrupt::detached(); + emu_d.set_interrupt_pin(PciInterruptPin::IntD, line_interrupt_d); + + emu_d.read_u32(0x3C, &mut val).unwrap(); + assert_eq!((val >> 8) & 0xFF, 4); // Interrupt pin should be 4 (INTD) + } } diff --git a/vm/devices/pci/pci_core/src/msi.rs b/vm/devices/pci/pci_core/src/msi.rs index 984467d45e..380028d236 100644 --- a/vm/devices/pci/pci_core/src/msi.rs +++ b/vm/devices/pci/pci_core/src/msi.rs @@ -3,6 +3,7 @@ //! Traits for working with MSI interrupts. +use inspect::Inspect; use parking_lot::Mutex; use std::sync::Arc; use std::sync::Weak; @@ -105,16 +106,37 @@ impl RegisterMsi for MsiInterruptSet { } /// A message-signaled interrupt. +#[derive(Debug, Inspect)] pub struct MsiInterrupt { state: Arc>, } +#[derive(Inspect)] struct MsiInterruptState { pending: bool, + #[inspect(with = "inspect_address_data")] address_data: Option<(u64, u32)>, + #[inspect(skip)] control: Option>, } +fn inspect_address_data(address_data: &Option<(u64, u32)>) -> String { + match address_data { + Some((address, data)) => format!("address: {:#x}, data: {:#x}", address, data), + None => "None".to_string(), + } +} + +impl std::fmt::Debug for MsiInterruptState { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("MsiInterruptState") + .field("pending", &self.pending) + .field("address_data", &self.address_data) + .field("control", &"") + .finish() + } +} + impl MsiInterrupt { /// Enables the interrupt. /// diff --git a/vm/devices/pci/pci_core/src/spec.rs b/vm/devices/pci/pci_core/src/spec.rs index 7643a8614b..787297eac7 100644 --- a/vm/devices/pci/pci_core/src/spec.rs +++ b/vm/devices/pci/pci_core/src/spec.rs @@ -194,6 +194,29 @@ pub mod cfg_space { use zerocopy::IntoBytes; use zerocopy::KnownLayout; + open_enum::open_enum! { + /// Common configuration space header registers shared between Type 0 and Type 1 headers. + /// + /// These registers appear at the same offsets in both header types and have the same + /// meaning and format. + /// + /// | Offset | Bits 31-24 | Bits 23-16 | Bits 15-8 | Bits 7-0 | + /// |--------|----------------|-------------|-------------|----------------------| + /// | 0x0 | Device ID | | Vendor ID | | + /// | 0x4 | Status | | Command | | + /// | 0x8 | Class code | | | Revision ID | + /// | 0x34 | Reserved | | | Capabilities Pointer | + pub enum CommonHeader: u16 { + DEVICE_VENDOR = 0x00, + STATUS_COMMAND = 0x04, + CLASS_REVISION = 0x08, + RESERVED_CAP_PTR = 0x34, + } + } + + /// Size of the common header portion shared by all PCI header types. + pub const COMMON_HEADER_SIZE: u16 = 0x10; + open_enum::open_enum! { /// Offsets into the type 00h configuration space header. /// @@ -385,12 +408,41 @@ pub mod caps { /// variants on an as-needed basis! pub enum CapabilityId: u8 { #![expect(missing_docs)] // self explanatory variants + MSI = 0x05, VENDOR_SPECIFIC = 0x09, PCI_EXPRESS = 0x10, MSIX = 0x11, } } + /// MSI + #[expect(missing_docs)] // primarily enums/structs with self-explanatory variants + pub mod msi { + open_enum::open_enum! { + /// Offsets into the MSI Capability Header + /// + /// Based on PCI Local Bus Specification Rev 3.0, Section 6.8.1 + /// + /// | Offset | Bits 31-24 | Bits 23-16 | Bits 15-8 | Bits 7-0 | + /// |-----------|---------------|---------------|---------------|-----------------------| + /// | Cap + 0x0 | Message Control | Next Pointer | Capability ID (0x05) | + /// | Cap + 0x4 | Message Address (32-bit or lower 32-bit of 64-bit) | + /// | Cap + 0x8 | Message Address Upper 32-bit (64-bit capable only) | + /// | Cap + 0xC | Message Data | | | | + /// | Cap + 0x10| Mask Bits (Per-vector masking capable only) | + /// | Cap + 0x14| Pending Bits (Per-vector masking capable only) | + pub enum MsiCapabilityHeader: u16 { + CONTROL_CAPS = 0x00, + MSG_ADDR_LO = 0x04, + MSG_ADDR_HI = 0x08, + MSG_DATA_32 = 0x08, // For 32-bit address capable + MSG_DATA_64 = 0x0C, // For 64-bit address capable + MASK_BITS = 0x10, // 64-bit + per-vector masking + PENDING_BITS = 0x14, // 64-bit + per-vector masking + } + } + } + /// MSI-X #[expect(missing_docs)] // primarily enums/structs with self-explanatory variants pub mod msix { @@ -432,6 +484,123 @@ pub mod caps { use zerocopy::IntoBytes; use zerocopy::KnownLayout; + /// PCIe Link Speed encoding values for use in Link Capabilities and other registers. + /// + /// Values are defined in PCIe Base Specification for the Max Link Speed field + /// in Link Capabilities Register and similar fields. + #[derive(Debug)] + #[repr(u32)] + pub enum LinkSpeed { + /// 2.5 GT/s link speed + Speed2_5GtS = 0b0001, + /// 5.0 GT/s link speed + Speed5_0GtS = 0b0010, + /// 8.0 GT/s link speed + Speed8_0GtS = 0b0011, + /// 16.0 GT/s link speed + Speed16_0GtS = 0b0100, + /// 32.0 GT/s link speed + Speed32_0GtS = 0b0101, + /// 64.0 GT/s link speed + Speed64_0GtS = 0b0110, + // All other encodings are reserved + } + + impl LinkSpeed { + pub const fn from_bits(bits: u32) -> Self { + match bits { + 0b0001 => LinkSpeed::Speed2_5GtS, + 0b0010 => LinkSpeed::Speed5_0GtS, + 0b0011 => LinkSpeed::Speed8_0GtS, + 0b0100 => LinkSpeed::Speed16_0GtS, + 0b0101 => LinkSpeed::Speed32_0GtS, + 0b0110 => LinkSpeed::Speed64_0GtS, + _ => unreachable!(), + } + } + + pub const fn into_bits(self) -> u32 { + self as u32 + } + } + + /// PCIe Supported Link Speeds Vector encoding values for use in Link Capabilities 2 register. + /// + /// Values are defined in PCIe Base Specification for the Supported Link Speeds Vector field + /// in Link Capabilities 2 Register. Each bit represents support for a specific generation. + #[derive(Debug)] + #[repr(u32)] + pub enum SupportedLinkSpeedsVector { + /// Support up to Gen 1 (2.5 GT/s) + UpToGen1 = 0b0000001, + /// Support up to Gen 2 (5.0 GT/s) + UpToGen2 = 0b0000011, + /// Support up to Gen 3 (8.0 GT/s) + UpToGen3 = 0b0000111, + /// Support up to Gen 4 (16.0 GT/s) + UpToGen4 = 0b0001111, + /// Support up to Gen 5 (32.0 GT/s) + UpToGen5 = 0b0011111, + /// Support up to Gen 6 (64.0 GT/s) + UpToGen6 = 0b0111111, + // All other encodings are reserved + } + + impl SupportedLinkSpeedsVector { + pub const fn from_bits(bits: u32) -> Self { + match bits { + 0b0000001 => SupportedLinkSpeedsVector::UpToGen1, + 0b0000011 => SupportedLinkSpeedsVector::UpToGen2, + 0b0000111 => SupportedLinkSpeedsVector::UpToGen3, + 0b0001111 => SupportedLinkSpeedsVector::UpToGen4, + 0b0011111 => SupportedLinkSpeedsVector::UpToGen5, + 0b0111111 => SupportedLinkSpeedsVector::UpToGen6, + _ => unreachable!(), + } + } + + pub const fn into_bits(self) -> u32 { + self as u32 + } + } + + /// PCIe Link Width encoding values for use in Link Capabilities and other registers. + /// + /// Values are defined in PCIe Base Specification for the Max Link Width field + /// in Link Capabilities Register and similar fields. + #[derive(Debug)] + #[repr(u32)] + pub enum LinkWidth { + /// x1 link width + X1 = 0b000001, + /// x2 link width + X2 = 0b000010, + /// x4 link width + X4 = 0b000100, + /// x8 link width + X8 = 0b001000, + /// x16 link width + X16 = 0b010000, + // All other encodings are reserved + } + + impl LinkWidth { + pub const fn from_bits(bits: u32) -> Self { + match bits { + 0b000001 => LinkWidth::X1, + 0b000010 => LinkWidth::X2, + 0b000100 => LinkWidth::X4, + 0b001000 => LinkWidth::X8, + 0b010000 => LinkWidth::X16, + _ => unreachable!(), + } + } + + pub const fn into_bits(self) -> u32 { + self as u32 + } + } + open_enum::open_enum! { /// Offsets into the PCI Express Capability Header /// diff --git a/vm/devices/pci/pcie/src/port.rs b/vm/devices/pci/pcie/src/port.rs index 39e92733c7..d1014b5ebe 100644 --- a/vm/devices/pci/pcie/src/port.rs +++ b/vm/devices/pci/pcie/src/port.rs @@ -7,8 +7,10 @@ use anyhow::bail; use chipset_device::io::IoResult; use inspect::Inspect; use pci_bus::GenericPciBusDevice; +use pci_core::capabilities::msi_cap::MsiCapability; use pci_core::capabilities::pci_express::PciExpressCapability; use pci_core::cfg_space_emu::ConfigSpaceType1Emulator; +use pci_core::msi::MsiInterruptSet; use pci_core::spec::caps::pci_express::DevicePortType; use pci_core::spec::hwid::HardwareIds; use std::sync::Arc; @@ -37,14 +39,47 @@ impl PcieDownstreamPort { hardware_ids: HardwareIds, port_type: DevicePortType, multi_function: bool, + hotplug: bool, + slot_number: Option, ) -> Self { + let port_name = name.into(); + tracing::info!( + "PcieDownstreamPort: creating new PCIe port '{}' with type {:?}, multi_function={}, hotplug={}, slot_number={:?}", + port_name, + port_type, + multi_function, + hotplug, + slot_number + ); + + let mut msi_set = MsiInterruptSet::new(); + // Create MSI capability with 1 message (multiple_message_capable=0), 64-bit addressing, no per-vector masking + let msi_capability = MsiCapability::new(0, true, false, &mut msi_set); + + tracing::info!( + "PcieDownstreamPort: '{}' creating config space with hardware_ids - vendor={:#x}, device={:#x}", + port_name, + hardware_ids.vendor_id, + hardware_ids.device_id + ); + + let pci_express_capability = if hotplug { + let slot_num = slot_number.unwrap_or(0); + PciExpressCapability::new(port_type, None).with_hotplug_support(slot_num) + } else { + PciExpressCapability::new(port_type, None) + }; + let cfg_space = ConfigSpaceType1Emulator::new( hardware_ids, - vec![Box::new(PciExpressCapability::new(port_type, None))], + vec![Box::new(pci_express_capability), Box::new(msi_capability)], ) .with_multi_function_bit(multi_function); + + tracing::info!("PcieDownstreamPort: '{}' created successfully", port_name); + Self { - name: name.into(), + name: port_name, cfg_space, link: None, } @@ -181,6 +216,10 @@ impl PcieDownstreamPort { // Connect the device to this port self.link = Some((device_name.into(), device)); + + // Set presence detect state to true when a device is connected + self.cfg_space.set_presence_detect_state(true); + return Ok(()); } @@ -188,3 +227,128 @@ impl PcieDownstreamPort { bail!("port name does not match") } } + +#[cfg(test)] +mod tests { + use super::*; + use chipset_device::io::IoResult; + use pci_bus::GenericPciBusDevice; + use pci_core::spec::hwid::HardwareIds; + + // Mock device for testing + struct MockDevice; + + impl GenericPciBusDevice for MockDevice { + fn pci_cfg_read(&mut self, _offset: u16, _value: &mut u32) -> Option { + None + } + + fn pci_cfg_write(&mut self, _offset: u16, _value: u32) -> Option { + None + } + + fn pci_cfg_read_forward( + &mut self, + _bus: u8, + _device_function: u8, + _offset: u16, + _value: &mut u32, + ) -> Option { + None + } + + fn pci_cfg_write_forward( + &mut self, + _bus: u8, + _device_function: u8, + _offset: u16, + _value: u32, + ) -> Option { + None + } + } + + #[test] + fn test_add_pcie_device_sets_presence_detect_state() { + use pci_core::spec::hwid::{ClassCode, ProgrammingInterface, Subclass}; + + // Create a port with hotplug support + let hardware_ids = HardwareIds { + vendor_id: 0x1234, + device_id: 0x5678, + revision_id: 0, + prog_if: ProgrammingInterface::NONE, + sub_class: Subclass::BRIDGE_PCI_TO_PCI, + base_class: ClassCode::BRIDGE, + type0_sub_vendor_id: 0, + type0_sub_system_id: 0, + }; + + let mut port = PcieDownstreamPort::new( + "test-port", + hardware_ids, + DevicePortType::RootPort, + false, + true, // Enable hotplug + Some(1), // Slot number 1 + ); + + // Initially, presence detect state should be 0 + let mut slot_status_val = 0u32; + let result = port.cfg_space.read_u32(0x58, &mut slot_status_val); // 0x40 (cap start) + 0x18 (slot control/status) + assert!(matches!(result, IoResult::Ok)); + let initial_presence_detect = (slot_status_val >> 22) & 0x1; // presence_detect_state is bit 6 of slot status + assert_eq!( + initial_presence_detect, 0, + "Initial presence detect state should be 0" + ); + + // Add a device to the port + let mock_device = Box::new(MockDevice); + let result = port.add_pcie_device("test-port", "mock-device", mock_device); + assert!(result.is_ok(), "Adding device should succeed"); + + // Check that presence detect state is now 1 + let result = port.cfg_space.read_u32(0x58, &mut slot_status_val); + assert!(matches!(result, IoResult::Ok)); + let present_presence_detect = (slot_status_val >> 22) & 0x1; + assert_eq!( + present_presence_detect, 1, + "Presence detect state should be 1 after adding device" + ); + } + + #[test] + fn test_add_pcie_device_without_hotplug() { + use pci_core::spec::hwid::{ClassCode, ProgrammingInterface, Subclass}; + + // Create a port without hotplug support + let hardware_ids = HardwareIds { + vendor_id: 0x1234, + device_id: 0x5678, + revision_id: 0, + prog_if: ProgrammingInterface::NONE, + sub_class: Subclass::BRIDGE_PCI_TO_PCI, + base_class: ClassCode::BRIDGE, + type0_sub_vendor_id: 0, + type0_sub_system_id: 0, + }; + + let mut port = PcieDownstreamPort::new( + "test-port", + hardware_ids, + DevicePortType::RootPort, + false, + false, // No hotplug + None, + ); + + // Add a device to the port (should not panic even without hotplug support) + let mock_device = Box::new(MockDevice); + let result = port.add_pcie_device("test-port", "mock-device", mock_device); + assert!( + result.is_ok(), + "Adding device should succeed even without hotplug support" + ); + } +} diff --git a/vm/devices/pci/pcie/src/root.rs b/vm/devices/pci/pcie/src/root.rs index d8ca97d0d9..97bb374d54 100644 --- a/vm/devices/pci/pcie/src/root.rs +++ b/vm/devices/pci/pcie/src/root.rs @@ -50,6 +50,8 @@ pub struct GenericPcieRootComplex { pub struct GenericPcieRootPortDefinition { /// The name of the root port. pub name: Arc, + /// Whether hotplug is enabled for this root port. + pub hotplug: bool, } /// A flat description of a PCIe switch without hierarchy. @@ -60,6 +62,8 @@ pub struct GenericSwitchDefinition { pub num_downstream_ports: u8, /// The parent port this switch is connected to. pub parent_port: Arc, + /// Whether hotplug is enabled for this switch. + pub hotplug: bool, } impl GenericSwitchDefinition { @@ -68,11 +72,13 @@ impl GenericSwitchDefinition { name: impl Into>, num_downstream_ports: u8, parent_port: impl Into>, + hotplug: bool, ) -> Self { Self { name: name.into(), num_downstream_ports, parent_port: parent_port.into(), + hotplug, } } } @@ -93,20 +99,48 @@ impl GenericPcieRootComplex { ecam_base: u64, ports: Vec, ) -> Self { + tracing::info!( + "GenericPcieRootComplex: creating new root complex - start_bus={}, end_bus={}, ecam_base={:#x}, num_ports={}", + start_bus, + end_bus, + ecam_base, + ports.len() + ); + let ecam_size = ecam_size_from_bus_numbers(start_bus, end_bus); + tracing::info!( + "GenericPcieRootComplex: ECAM size calculated as {:#x}", + ecam_size + ); + let mut ecam = register_mmio.new_io_region("ecam", ecam_size); ecam.map(ecam_base); + tracing::info!( + "GenericPcieRootComplex: ECAM region mapped at base {:#x}", + ecam_base + ); let port_map: HashMap, RootPort)> = ports .into_iter() .enumerate() .map(|(i, definition)| { let device_number: u8 = (i << BDF_DEVICE_SHIFT).try_into().expect("too many ports"); - let emulator = RootPort::new(definition.name.clone()); - (device_number, (definition.name, emulator)) + tracing::info!( + "GenericPcieRootComplex: creating port {} at device number {:#x} with name '{}'", + i, device_number, definition.name + ); + // Use the device number as the slot number for hotpluggable ports + let slot_number = if definition.hotplug { Some((device_number as u32) + 1) } else { None }; + let root_port = RootPort::new(definition.name.clone(), definition.hotplug, slot_number); + (device_number, (definition.name, root_port)) }) .collect(); + tracing::info!( + "GenericPcieRootComplex: root complex created successfully with {} ports", + port_map.len() + ); + Self { start_bus, end_bus, @@ -122,20 +156,65 @@ impl GenericPcieRootComplex { name: impl AsRef, dev: Box, ) -> Result<(), Arc> { - let (_, root_port) = self - .ports - .get_mut(&port) - .expect("caller must pass port number returned by downstream_ports()"); - root_port.connect_device(name, dev)?; - Ok(()) + tracing::info!( + "GenericPcieRootComplex: adding PCIe device '{}' to port {:#x}", + name.as_ref(), + port + ); + + let (port_name, root_port) = self.ports.get_mut(&port).ok_or_else(|| -> Arc { + tracing::error!( + "GenericPcieRootComplex: port {:#x} not found for device '{}'", + port, + name.as_ref() + ); + format!("Port {:#x} not found", port).into() + })?; + + tracing::info!( + "GenericPcieRootComplex: connecting device '{}' to root port '{}' at port {:#x}", + name.as_ref(), + port_name, + port + ); + + match root_port.connect_device(name, dev) { + Ok(()) => { + tracing::info!( + "GenericPcieRootComplex: successfully connected device to port {:#x}", + port + ); + Ok(()) + } + Err(existing_device) => { + tracing::warn!( + "GenericPcieRootComplex: failed to connect device to port {:#x}, existing device: '{}'", + port, + existing_device + ); + Err(existing_device) + } + } } /// Enumerate the downstream ports of the root complex. pub fn downstream_ports(&self) -> Vec<(u8, Arc)> { - self.ports + let ports: Vec<(u8, Arc)> = self + .ports .iter() .map(|(port, (name, _))| (*port, name.clone())) - .collect() + .collect(); + + tracing::info!( + "GenericPcieRootComplex: enumerating {} downstream ports: {:?}", + ports.len(), + ports + .iter() + .map(|(port, name)| format!("{:#x}:{}", port, name)) + .collect::>() + ); + + ports } /// Returns the size of the ECAM MMIO region this root complex is emulating. @@ -198,9 +277,19 @@ impl ChangeDeviceState for GenericPcieRootComplex { async fn stop(&mut self) {} async fn reset(&mut self) { - for (_, (_, port)) in self.ports.iter_mut() { + tracing::info!( + "GenericPcieRootComplex: starting reset for {} ports", + self.ports.len() + ); + for (device_num, (name, port)) in self.ports.iter_mut() { + tracing::info!( + "GenericPcieRootComplex: resetting port {} ({})", + device_num, + name + ); port.port.cfg_space.reset(); } + tracing::info!("GenericPcieRootComplex: reset completed"); } } @@ -240,6 +329,12 @@ impl MmioIntercept for GenericPcieRootComplex { fn mmio_read(&mut self, addr: u64, data: &mut [u8]) -> IoResult { validate_ecam_intercept!(addr, data); + tracing::trace!( + "GenericPcieRootComplex: ECAM read at addr={:#x}, len={}", + addr, + data.len() + ); + // N.B. Emulators internally only support 4-byte aligned accesses to // 4-byte registers, but the guest can use 1-, 2-, or 4 byte memory // instructions to access ECAM. This function reads the 4-byte aligned @@ -256,9 +351,19 @@ impl MmioIntercept for GenericPcieRootComplex { tracelimit::warn_ratelimited!("unroutable config space access"); } DecodedEcamAccess::InternalBus(port, cfg_offset) => { + tracing::trace!( + "GenericPcieRootComplex: reading internal bus cfg_offset={:#x}", + cfg_offset + ); check_result!(port.port.cfg_space.read_u32(cfg_offset, &mut dword_value)); } DecodedEcamAccess::DownstreamPort(port, bus_number, device_function, cfg_offset) => { + tracing::trace!( + "GenericPcieRootComplex: reading downstream port bus={}, dev_fn={}, cfg_offset={:#x}", + bus_number, + device_function, + cfg_offset + ); check_result!(port.forward_cfg_read( &bus_number, &device_function, @@ -273,12 +378,25 @@ impl MmioIntercept for GenericPcieRootComplex { &dword_value.as_bytes() [byte_offset_within_dword..byte_offset_within_dword + data.len()], ); + + tracing::trace!( + "GenericPcieRootComplex: ECAM read addr={:#x} -> value={:#x}", + addr, + dword_value + ); IoResult::Ok } fn mmio_write(&mut self, addr: u64, data: &[u8]) -> IoResult { validate_ecam_intercept!(addr, data); + tracing::trace!( + "GenericPcieRootComplex: ECAM write at addr={:#x}, len={}, data={:?}", + addr, + data.len(), + data + ); + // N.B. Emulators internally only support 4-byte aligned accesses to // 4-byte registers, but the guest can use 1-, 2-, or 4-byte memory // instructions to access ECAM. If the guest is using a 1- or 2-byte @@ -307,6 +425,12 @@ impl MmioIntercept for GenericPcieRootComplex { } }; + tracing::trace!( + "GenericPcieRootComplex: ECAM write dword_aligned_addr={:#x}, write_dword={:#x}", + dword_aligned_addr, + write_dword + ); + match self.decode_ecam_access(dword_aligned_addr) { DecodedEcamAccess::UnexpectedIntercept => { tracing::error!("unexpected intercept at address 0x{:16x}", addr); @@ -315,9 +439,21 @@ impl MmioIntercept for GenericPcieRootComplex { tracelimit::warn_ratelimited!("unroutable config space access"); } DecodedEcamAccess::InternalBus(port, cfg_offset) => { + tracing::trace!( + "GenericPcieRootComplex: writing internal bus cfg_offset={:#x}, val={:#x}", + cfg_offset, + write_dword + ); check_result!(port.port.cfg_space.write_u32(cfg_offset, write_dword)); } DecodedEcamAccess::DownstreamPort(port, bus_number, device_function, cfg_offset) => { + tracing::trace!( + "GenericPcieRootComplex: writing downstream port bus={}, dev_fn={}, cfg_offset={:#x}, val={:#x}", + bus_number, + device_function, + cfg_offset, + write_dword + ); check_result!(port.forward_cfg_write( &bus_number, &device_function, @@ -340,7 +476,10 @@ struct RootPort { impl RootPort { /// Constructs a new [`RootPort`] emulator. - pub fn new(name: impl Into>) -> Self { + pub fn new(name: impl Into>, hotplug: bool, slot_number: Option) -> Self { + let name_str = name.into(); + tracing::info!("RootPort: creating new root port '{}'", name_str); + let hardware_ids = HardwareIds { vendor_id: VENDOR_ID, device_id: ROOT_PORT_DEVICE_ID, @@ -351,14 +490,25 @@ impl RootPort { type0_sub_vendor_id: 0, type0_sub_system_id: 0, }; - Self { - port: PcieDownstreamPort::new( - name.into().to_string(), - hardware_ids, - DevicePortType::RootPort, - false, - ), - } + + tracing::info!( + "RootPort: hardware_ids configured - vendor={:#x}, device={:#x}", + hardware_ids.vendor_id, + hardware_ids.device_id + ); + + let port = PcieDownstreamPort::new( + name_str.to_string(), + hardware_ids, + DevicePortType::RootPort, + false, + hotplug, + slot_number, + ); + + tracing::info!("RootPort: '{}' created successfully", name_str); + + Self { port } } /// Try to connect a PCIe device, returning an existing device name if the @@ -368,16 +518,42 @@ impl RootPort { name: impl AsRef, dev: Box, ) -> Result<(), Arc> { + let device_name = name.as_ref(); let port_name = self.port.name.clone(); - match self.port.add_pcie_device(&port_name, name.as_ref(), dev) { - Ok(()) => Ok(()), + + tracing::info!( + "RootPort: '{}' attempting to connect device '{}'", + port_name, + device_name + ); + + match self.port.add_pcie_device(&port_name, device_name, dev) { + Ok(()) => { + tracing::info!( + "RootPort: '{}' successfully connected device '{}'", + port_name, + device_name + ); + Ok(()) + } Err(_error) => { // If the connection failed, it means the port is already occupied // We need to get the name of the existing device if let Some((existing_name, _)) = &self.port.link { + tracing::warn!( + "RootPort: '{}' failed to connect device '{}', port already occupied by '{}'", + port_name, + device_name, + existing_name + ); Err(existing_name.clone()) } else { // This shouldn't happen if add_pcie_device works correctly + tracing::error!( + "RootPort: '{}' connection failed for device '{}' but no existing device found", + port_name, + device_name + ); panic!("Port connection failed but no existing device found") } } @@ -443,6 +619,7 @@ mod tests { let port_defs = (0..port_count) .map(|i| GenericPcieRootPortDefinition { name: format!("test-port-{}", i).into(), + hotplug: false, }) .collect(); @@ -693,7 +870,7 @@ mod tests { #[test] fn test_root_port_invalid_bus_range_handling() { - let mut root_port = RootPort::new("test-port"); + let mut root_port = RootPort::new("test-port", false, None); // Don't configure bus numbers, so the range should be 0..=0 (invalid) let bus_range = root_port.port.cfg_space.assigned_bus_range(); diff --git a/vm/devices/pci/pcie/src/switch.rs b/vm/devices/pci/pcie/src/switch.rs index e8bb8daead..c2e64a0eb0 100644 --- a/vm/devices/pci/pcie/src/switch.rs +++ b/vm/devices/pci/pcie/src/switch.rs @@ -87,12 +87,20 @@ pub struct DownstreamSwitchPort { impl DownstreamSwitchPort { /// Constructs a new [`DownstreamSwitchPort`] emulator. - pub fn new(name: impl Into>) -> Self { - Self::new_with_multi_function(name, false) - } - - /// Constructs a new [`DownstreamSwitchPort`] emulator with multi-function flag. - pub fn new_with_multi_function(name: impl Into>, multi_function: bool) -> Self { + /// + /// # Arguments + /// * `name` - The name for this downstream switch port + /// * `multi_function` - Whether this port should have the multi-function flag set (default: false) + /// * `hotplug` - Whether this port should support hotplug (default: false) + /// * `slot_number` - The slot number for hotpluggable ports (only used if hotplug is true) + pub fn new( + name: impl Into>, + multi_function: Option, + hotplug: Option, + slot_number: Option, + ) -> Self { + let multi_function = multi_function.unwrap_or(false); + let hotplug = hotplug.unwrap_or(false); let hardware_ids = HardwareIds { vendor_id: VENDOR_ID, device_id: DOWNSTREAM_SWITCH_PORT_DEVICE_ID, @@ -103,14 +111,17 @@ impl DownstreamSwitchPort { type0_sub_vendor_id: 0, type0_sub_system_id: 0, }; - Self { - port: PcieDownstreamPort::new( - name.into().to_string(), - hardware_ids, - DevicePortType::DownstreamSwitchPort, - multi_function, - ), - } + + let port = PcieDownstreamPort::new( + name.into().to_string(), + hardware_ids, + DevicePortType::DownstreamSwitchPort, + multi_function, + hotplug, + slot_number, + ); + + Self { port } } /// Get a reference to the configuration space emulator. @@ -122,11 +133,6 @@ impl DownstreamSwitchPort { pub fn cfg_space_mut(&mut self) -> &mut ConfigSpaceType1Emulator { &mut self.port.cfg_space } - - /// Get a mutable reference to the underlying PCIe port. - pub fn port_mut(&mut self) -> &mut PcieDownstreamPort { - &mut self.port - } } /// A PCI Express switch definition used for creating switch instances. @@ -135,7 +141,9 @@ pub struct GenericPcieSwitchDefinition { pub name: Arc, /// The number of downstream ports to create. /// TODO: implement physical slot number, link and slot stuff - pub downstream_port_count: usize, + pub downstream_port_count: u8, + /// Whether hotplug is enabled for this switch's downstream ports. + pub hotplug: bool, } /// A PCI Express switch emulator that implements a complete switch with upstream and downstream ports. @@ -168,11 +176,19 @@ impl GenericPcieSwitch { let downstream_ports = (0..definition.downstream_port_count) .map(|i| { let port_name = format!("{}-downstream-{}", definition.name, i); - let port = DownstreamSwitchPort::new_with_multi_function( + // Use the port index as the slot number for hotpluggable ports + let slot_number = if definition.hotplug { + Some((i as u32) + 1) + } else { + None + }; + let port = DownstreamSwitchPort::new( port_name.clone(), - multi_function, + Some(multi_function), + Some(definition.hotplug), + slot_number, ); - (i as u8, (port_name.into(), port)) + (i, (port_name.into(), port)) }) .collect(); @@ -473,7 +489,7 @@ mod tests { #[test] fn test_downstream_switch_port_creation() { - let port = DownstreamSwitchPort::new("test-downstream-port"); + let port = DownstreamSwitchPort::new("test-downstream-port", None, None, None); assert!(port.port.link.is_none()); // Verify that we can read the vendor/device ID from config space @@ -486,11 +502,57 @@ mod tests { assert_eq!(vendor_device_id, expected); } + #[test] + fn test_downstream_switch_port_multi_function_options() { + // Test with default multi_function (false) + let port_default = DownstreamSwitchPort::new("test-port-default", None, None, None); + let mut header_type_value: u32 = 0; + port_default + .cfg_space() + .read_u32(0x0C, &mut header_type_value) + .unwrap(); + let header_type_field = (header_type_value >> 16) & 0xFF; + assert_eq!( + header_type_field & 0x80, + 0x00, + "Multi-function bit should NOT be set with None parameter" + ); + + // Test with explicit multi_function false + let port_false = DownstreamSwitchPort::new("test-port-false", Some(false), None, None); + let mut header_type_value_false: u32 = 0; + port_false + .cfg_space() + .read_u32(0x0C, &mut header_type_value_false) + .unwrap(); + let header_type_field_false = (header_type_value_false >> 16) & 0xFF; + assert_eq!( + header_type_field_false & 0x80, + 0x00, + "Multi-function bit should NOT be set with Some(false)" + ); + + // Test with explicit multi_function true + let port_true = DownstreamSwitchPort::new("test-port-true", Some(true), None, None); + let mut header_type_value_true: u32 = 0; + port_true + .cfg_space() + .read_u32(0x0C, &mut header_type_value_true) + .unwrap(); + let header_type_field_true = (header_type_value_true >> 16) & 0xFF; + assert_eq!( + header_type_field_true & 0x80, + 0x80, + "Multi-function bit should be set with Some(true)" + ); + } + #[test] fn test_switch_creation() { let definition = GenericPcieSwitchDefinition { name: "test-switch".into(), downstream_port_count: 3, + hotplug: false, }; let switch = GenericPcieSwitch::new(definition); @@ -521,6 +583,7 @@ mod tests { let definition = GenericPcieSwitchDefinition { name: "test-switch".into(), downstream_port_count: 2, + hotplug: false, }; let mut switch = GenericPcieSwitch::new(definition); @@ -566,6 +629,7 @@ mod tests { let definition = GenericPcieSwitchDefinition { name: "test-switch".into(), downstream_port_count: 2, + hotplug: false, }; let mut switch = GenericPcieSwitch::new(definition); @@ -598,6 +662,7 @@ mod tests { let definition = GenericPcieSwitchDefinition { name: "test-switch".into(), downstream_port_count: 4, + hotplug: false, }; let mut switch = GenericPcieSwitch::new(definition); @@ -626,6 +691,7 @@ mod tests { let definition = GenericPcieSwitchDefinition { name: "default-switch".into(), downstream_port_count: 4, + hotplug: false, }; let switch = GenericPcieSwitch::new(definition); assert_eq!(switch.name().as_ref(), "default-switch"); @@ -637,6 +703,7 @@ mod tests { let definition = GenericPcieSwitchDefinition { name: "test-switch".into(), downstream_port_count: 16, + hotplug: false, }; let switch = GenericPcieSwitch::new(definition); assert_eq!(switch.downstream_ports().len(), 16); @@ -647,6 +714,7 @@ mod tests { let definition = GenericPcieSwitchDefinition { name: "test-switch".into(), downstream_port_count: 3, + hotplug: false, }; let mut switch = GenericPcieSwitch::new(definition); @@ -689,6 +757,7 @@ mod tests { let definition = GenericPcieSwitchDefinition { name: "test-switch".into(), downstream_port_count: 2, + hotplug: false, }; let mut switch = GenericPcieSwitch::new(definition); @@ -713,6 +782,7 @@ mod tests { let definition = GenericPcieSwitchDefinition { name: "test-switch".into(), downstream_port_count: 2, + hotplug: false, }; let mut switch = GenericPcieSwitch::new(definition); @@ -751,6 +821,7 @@ mod tests { let multi_port_definition = GenericPcieSwitchDefinition { name: "multi-port-switch".into(), downstream_port_count: 3, + hotplug: false, }; let multi_port_switch = GenericPcieSwitch::new(multi_port_definition); @@ -788,6 +859,7 @@ mod tests { let single_port_definition = GenericPcieSwitchDefinition { name: "single-port-switch".into(), downstream_port_count: 1, + hotplug: false, }; let single_port_switch = GenericPcieSwitch::new(single_port_definition); @@ -821,4 +893,28 @@ mod tests { } } } + + #[test] + fn test_hotplug_support() { + // Test hotplug disabled + let definition_no_hotplug = GenericPcieSwitchDefinition { + name: "test-switch-no-hotplug".into(), + downstream_port_count: 1, + hotplug: false, + }; + let switch_no_hotplug = GenericPcieSwitch::new(definition_no_hotplug); + assert_eq!(switch_no_hotplug.name().as_ref(), "test-switch-no-hotplug"); + + // Test hotplug enabled + let definition_with_hotplug = GenericPcieSwitchDefinition { + name: "test-switch-with-hotplug".into(), + downstream_port_count: 1, + hotplug: true, + }; + let switch_with_hotplug = GenericPcieSwitch::new(definition_with_hotplug); + assert_eq!( + switch_with_hotplug.name().as_ref(), + "test-switch-with-hotplug" + ); + } } diff --git a/vmm_tests/vmm_tests/tests/tests/multiarch/pcie.rs b/vmm_tests/vmm_tests/tests/tests/multiarch/pcie.rs index fa0ab2d02f..628f76e9dc 100644 --- a/vmm_tests/vmm_tests/tests/tests/multiarch/pcie.rs +++ b/vmm_tests/vmm_tests/tests/tests/multiarch/pcie.rs @@ -124,10 +124,22 @@ async fn pcie_root_emulation(config: PetriVmBuilder) -> any low_mmio_size: 1024 * 1024, high_mmio_size: 1024 * 1024 * 1024, ports: vec![ - PcieRootPortConfig { name: "rp0".into() }, - PcieRootPortConfig { name: "rp1".into() }, - PcieRootPortConfig { name: "rp2".into() }, - PcieRootPortConfig { name: "rp3".into() }, + PcieRootPortConfig { + name: "rp0".into(), + hotplug: false, + }, + PcieRootPortConfig { + name: "rp1".into(), + hotplug: false, + }, + PcieRootPortConfig { + name: "rp2".into(), + hotplug: false, + }, + PcieRootPortConfig { + name: "rp3".into(), + hotplug: false, + }, ], }) })