diff --git a/devices/src/pvmemcontrol.rs b/devices/src/pvmemcontrol.rs index 4adfc6b3c..832d83b08 100644 --- a/devices/src/pvmemcontrol.rs +++ b/devices/src/pvmemcontrol.rs @@ -698,10 +698,12 @@ impl PciDevice for PvmemcontrolPciDevice { reg_idx: usize, offset: u64, data: &[u8], - ) -> Option> { - self.configuration - .write_config_register(reg_idx, offset, data); - None + ) -> (Option, Option>) { + ( + self.configuration + .write_config_register(reg_idx, offset, data), + None, + ) } fn read_config_register(&mut self, reg_idx: usize) -> u32 { @@ -716,14 +718,6 @@ impl PciDevice for PvmemcontrolPciDevice { Some(self.id.clone()) } - fn detect_bar_reprogramming( - &mut self, - reg_idx: usize, - data: &[u8], - ) -> Option { - self.configuration.detect_bar_reprogramming(reg_idx, data) - } - fn allocate_bars( &mut self, _allocator: &Arc>, diff --git a/devices/src/pvpanic.rs b/devices/src/pvpanic.rs index ca86a4ae0..dd7cb8329 100644 --- a/devices/src/pvpanic.rs +++ b/devices/src/pvpanic.rs @@ -88,7 +88,11 @@ impl PvPanicDevice { ); let command: [u8; 2] = [0x03, 0x01]; - configuration.write_config_register(1, 0, &command); + let bar_reprogram = configuration.write_config_register(1, 0, &command); + assert!( + bar_reprogram.is_none(), + "No bar reprogrammig is expected from writing to the COMMAND register" + ); let state: Option = snapshot .as_ref() @@ -156,24 +160,18 @@ impl PciDevice for PvPanicDevice { reg_idx: usize, offset: u64, data: &[u8], - ) -> Option> { - self.configuration - .write_config_register(reg_idx, offset, data); - None + ) -> (Option, Option>) { + ( + self.configuration + .write_config_register(reg_idx, offset, data), + None, + ) } fn read_config_register(&mut self, reg_idx: usize) -> u32 { self.configuration.read_reg(reg_idx) } - fn detect_bar_reprogramming( - &mut self, - reg_idx: usize, - data: &[u8], - ) -> Option { - self.configuration.detect_bar_reprogramming(reg_idx, data) - } - fn allocate_bars( &mut self, _allocator: &Arc>, diff --git a/pci/src/bus.rs b/pci/src/bus.rs index aa0bc9547..04284b435 100644 --- a/pci/src/bus.rs +++ b/pci/src/bus.rs @@ -15,7 +15,7 @@ use vm_device::{Bus, BusDevice, BusDeviceSync}; use crate::configuration::{ PciBarRegionType, PciBridgeSubclass, PciClassCode, PciConfiguration, PciHeaderType, }; -use crate::device::{DeviceRelocation, Error as PciDeviceError, PciDevice}; +use crate::device::{BarReprogrammingParams, DeviceRelocation, Error as PciDeviceError, PciDevice}; use crate::PciBarConfiguration; const VENDOR_ID_INTEL: u16 = 0x8086; @@ -81,9 +81,11 @@ impl PciDevice for PciRoot { reg_idx: usize, offset: u64, data: &[u8], - ) -> Option> { - self.config.write_config_register(reg_idx, offset, data); - None + ) -> (Option, Option>) { + ( + self.config.write_config_register(reg_idx, offset, data), + None, + ) } fn read_config_register(&mut self, reg_idx: usize) -> u32 { @@ -256,9 +258,11 @@ impl PciConfigIo { if let Some(d) = pci_bus.devices.get(&(device as u32)) { let mut device = d.lock().unwrap(); - // Find out if one of the device's BAR is being reprogrammed, and - // reprogram it if needed. - if let Some(params) = device.detect_bar_reprogramming(register, data) { + // Update the register value + let (bar_reprogram, ret) = device.write_config_register(register, offset, data); + + // Move the device's BAR if needed + if let Some(params) = bar_reprogram { if let Err(e) = pci_bus.device_reloc.move_bar( params.old_base, params.new_base, @@ -273,8 +277,7 @@ impl PciConfigIo { } } - // Update the register value - device.write_config_register(register, offset, data) + ret } else { None } @@ -380,9 +383,11 @@ impl PciConfigMmio { if let Some(d) = pci_bus.devices.get(&(device as u32)) { let mut device = d.lock().unwrap(); - // Find out if one of the device's BAR is being reprogrammed, and - // reprogram it if needed. - if let Some(params) = device.detect_bar_reprogramming(register, data) { + // Update the register value + let (bar_reprogram, _) = device.write_config_register(register, offset, data); + + // Move the device's BAR if needed + if let Some(params) = bar_reprogram { if let Err(e) = pci_bus.device_reloc.move_bar( params.old_base, params.new_base, @@ -396,9 +401,6 @@ impl PciConfigMmio { ); } } - - // Update the register value - device.write_config_register(register, offset, data); } } } diff --git a/pci/src/configuration.rs b/pci/src/configuration.rs index f1ebb4ca2..94bd83222 100644 --- a/pci/src/configuration.rs +++ b/pci/src/configuration.rs @@ -907,9 +907,14 @@ impl PciConfiguration { (next + 3) & !3 } - pub fn write_config_register(&mut self, reg_idx: usize, offset: u64, data: &[u8]) { + pub fn write_config_register( + &mut self, + reg_idx: usize, + offset: u64, + data: &[u8], + ) -> Option { if offset as usize + data.len() > 4 { - return; + return None; } // Handle potential write to MSI-X message control register @@ -938,13 +943,15 @@ impl PciConfiguration { 4 => self.write_reg(reg_idx, LittleEndian::read_u32(data)), _ => (), } + + self.detect_bar_reprogramming(reg_idx, data) } pub fn read_config_register(&self, reg_idx: usize) -> u32 { self.read_reg(reg_idx) } - pub fn detect_bar_reprogramming( + fn detect_bar_reprogramming( &mut self, reg_idx: usize, data: &[u8], diff --git a/pci/src/device.rs b/pci/src/device.rs index d74b46f73..1e834ab0e 100644 --- a/pci/src/device.rs +++ b/pci/src/device.rs @@ -87,18 +87,10 @@ pub trait PciDevice: Send { reg_idx: usize, offset: u64, data: &[u8], - ) -> Option>; + ) -> (Option, Option>); /// Gets a register from the configuration space. /// * `reg_idx` - The index of the config register to read. fn read_config_register(&mut self, reg_idx: usize) -> u32; - /// Detects if a BAR is being reprogrammed. - fn detect_bar_reprogramming( - &mut self, - _reg_idx: usize, - _data: &[u8], - ) -> Option { - None - } /// Reads from a BAR region mapped into the device. /// * `addr` - The guest address inside the BAR. /// * `data` - Filled with the data from `addr`. diff --git a/pci/src/vfio.rs b/pci/src/vfio.rs index 267f92eeb..41965e445 100644 --- a/pci/src/vfio.rs +++ b/pci/src/vfio.rs @@ -1242,7 +1242,7 @@ impl VfioCommon { reg_idx: usize, offset: u64, data: &[u8], - ) -> Option> { + ) -> (Option, Option>) { // When the guest wants to write to a BAR, we trap it into // our local configuration space. We're not reprogramming // VFIO device. @@ -1252,9 +1252,11 @@ impl VfioCommon { // We keep our local cache updated with the BARs. // We'll read it back from there when the guest is asking // for BARs (see read_config_register()). - self.configuration - .write_config_register(reg_idx, offset, data); - return None; + return ( + self.configuration + .write_config_register(reg_idx, offset, data), + None, + ); } let reg = (reg_idx * PCI_CONFIG_REGISTER_SIZE) as u64; @@ -1290,7 +1292,7 @@ impl VfioCommon { // to the device region to update the MSI Enable bit. self.vfio_wrapper.write_config((reg + offset) as u32, data); - None + (None, None) } pub(crate) fn read_config_register(&mut self, reg_idx: usize) -> u32 { @@ -1850,7 +1852,7 @@ impl PciDevice for VfioPciDevice { reg_idx: usize, offset: u64, data: &[u8], - ) -> Option> { + ) -> (Option, Option>) { self.common.write_config_register(reg_idx, offset, data) } @@ -1858,16 +1860,6 @@ impl PciDevice for VfioPciDevice { self.common.read_config_register(reg_idx) } - fn detect_bar_reprogramming( - &mut self, - reg_idx: usize, - data: &[u8], - ) -> Option { - self.common - .configuration - .detect_bar_reprogramming(reg_idx, data) - } - fn read_bar(&mut self, base: u64, offset: u64, data: &mut [u8]) { self.common.read_bar(base, offset, data) } diff --git a/pci/src/vfio_user.rs b/pci/src/vfio_user.rs index b0e41a02c..017864883 100644 --- a/pci/src/vfio_user.rs +++ b/pci/src/vfio_user.rs @@ -426,22 +426,12 @@ impl PciDevice for VfioUserPciDevice { self } - fn detect_bar_reprogramming( - &mut self, - reg_idx: usize, - data: &[u8], - ) -> Option { - self.common - .configuration - .detect_bar_reprogramming(reg_idx, data) - } - fn write_config_register( &mut self, reg_idx: usize, offset: u64, data: &[u8], - ) -> Option> { + ) -> (Option, Option>) { self.common.write_config_register(reg_idx, offset, data) } diff --git a/virtio-devices/src/transport/pci_device.rs b/virtio-devices/src/transport/pci_device.rs index c06b5cb1b..06a21c9fe 100644 --- a/virtio-devices/src/transport/pci_device.rs +++ b/virtio-devices/src/transport/pci_device.rs @@ -912,7 +912,7 @@ impl PciDevice for VirtioPciDevice { reg_idx: usize, offset: u64, data: &[u8], - ) -> Option> { + ) -> (Option, Option>) { // Handle the special case where the capability VIRTIO_PCI_CAP_PCI_CFG // is accessed. This capability has a special meaning as it allows the // guest to access other capabilities without mapping the PCI BAR. @@ -922,11 +922,13 @@ impl PciDevice for VirtioPciDevice { <= self.cap_pci_cfg_info.offset + self.cap_pci_cfg_info.cap.bytes().len() { let offset = base + offset as usize - self.cap_pci_cfg_info.offset; - self.write_cap_pci_cfg(offset, data) + (None, self.write_cap_pci_cfg(offset, data)) } else { - self.configuration - .write_config_register(reg_idx, offset, data); - None + ( + self.configuration + .write_config_register(reg_idx, offset, data), + None, + ) } } @@ -947,14 +949,6 @@ impl PciDevice for VirtioPciDevice { } } - fn detect_bar_reprogramming( - &mut self, - reg_idx: usize, - data: &[u8], - ) -> Option { - self.configuration.detect_bar_reprogramming(reg_idx, data) - } - fn allocate_bars( &mut self, _allocator: &Arc>,