pci: Keep detect_bar_reprogramming internal to PciConfiguration

A BAR reprogramming of a PCI device will only happen when the (guest)
kernel write to its PCI config space, e.g. the detection of bar
reprogramming (`detect_bar_repgraomming()`) can be embedded to the PCI
config space write (`write_config_register()`). It simplifies APIs
exposed by the `struct PciConfiguration` and `trait PciDevice`. It also
prepares for easier handling of pending bar reprogramming when the MSE
bit of the COMMAND register is not enabled at the time of changing BAR
registers.

See: https://github.com/cloud-hypervisor/cloud-hypervisor/issues/7027#issuecomment-2853642959

Signed-off-by: Bo Chen <bchen@crusoe.ai>

devices/src/pvmemcontrol.rs

@@ -698,10 +698,12 @@ impl PciDevice for PvmemcontrolPciDevice {
         reg_idx: usize,
         offset: u64,
         data: &[u8],
-    ) -> Option<Arc<Barrier>> {
+    ) -> (Option<BarReprogrammingParams>, Option<Arc<Barrier>>) {
-        self.configuration
+        (
-            .write_config_register(reg_idx, offset, data);
+            self.configuration
-        None
+                .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<BarReprogrammingParams> {
-        self.configuration.detect_bar_reprogramming(reg_idx, data)
-    }
-
     fn allocate_bars(
         &mut self,
         _allocator: &Arc<Mutex<SystemAllocator>>,

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<PvPanicDeviceState> = snapshot
             .as_ref()
@@ -156,24 +160,18 @@ impl PciDevice for PvPanicDevice {
         reg_idx: usize,
         offset: u64,
         data: &[u8],
-    ) -> Option<Arc<Barrier>> {
+    ) -> (Option<BarReprogrammingParams>, Option<Arc<Barrier>>) {
-        self.configuration
+        (
-            .write_config_register(reg_idx, offset, data);
+            self.configuration
-        None
+                .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<BarReprogrammingParams> {
-        self.configuration.detect_bar_reprogramming(reg_idx, data)
-    }
-
     fn allocate_bars(
         &mut self,
         _allocator: &Arc<Mutex<SystemAllocator>>,

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<Arc<Barrier>> {
+    ) -> (Option<BarReprogrammingParams>, Option<Arc<Barrier>>) {
-        self.config.write_config_register(reg_idx, offset, data);
+        (
-        None
+            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
+            // Update the register value
-            // reprogram it if needed.
+            let (bar_reprogram, ret) = device.write_config_register(register, offset, data);
-            if let Some(params) = device.detect_bar_reprogramming(register, 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
+            ret
-            device.write_config_register(register, offset, data)
         } 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
+            // Update the register value
-            // reprogram it if needed.
+            let (bar_reprogram, _) = device.write_config_register(register, offset, data);
-            if let Some(params) = device.detect_bar_reprogramming(register, 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);
         }
     }
 }

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<BarReprogrammingParams> {
         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],

pci/src/device.rs

@@ -87,18 +87,10 @@ pub trait PciDevice: Send {
         reg_idx: usize,
         offset: u64,
         data: &[u8],
-    ) -> Option<Arc<Barrier>>;
+    ) -> (Option<BarReprogrammingParams>, Option<Arc<Barrier>>);
     /// 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<BarReprogrammingParams> {
-        None
-    }
     /// Reads from a BAR region mapped into the device.
     /// * `addr` - The guest address inside the BAR.
     /// * `data` - Filled with the data from `addr`.

pci/src/vfio.rs

@@ -1242,7 +1242,7 @@ impl VfioCommon {
         reg_idx: usize,
         offset: u64,
         data: &[u8],
-    ) -> Option<Arc<Barrier>> {
+    ) -> (Option<BarReprogrammingParams>, Option<Arc<Barrier>>) {
         // 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
+            return (
-                .write_config_register(reg_idx, offset, data);
+                self.configuration
-            return None;
+                    .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<Arc<Barrier>> {
+    ) -> (Option<BarReprogrammingParams>, Option<Arc<Barrier>>) {
         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<BarReprogrammingParams> {
-        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)
     }

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<BarReprogrammingParams> {
-        self.common
-            .configuration
-            .detect_bar_reprogramming(reg_idx, data)
-    }
-
     fn write_config_register(
         &mut self,
         reg_idx: usize,
         offset: u64,
         data: &[u8],
-    ) -> Option<Arc<Barrier>> {
+    ) -> (Option<BarReprogrammingParams>, Option<Arc<Barrier>>) {
         self.common.write_config_register(reg_idx, offset, data)
     }
 

virtio-devices/src/transport/pci_device.rs

@@ -912,7 +912,7 @@ impl PciDevice for VirtioPciDevice {
         reg_idx: usize,
         offset: u64,
         data: &[u8],
-    ) -> Option<Arc<Barrier>> {
+    ) -> (Option<BarReprogrammingParams>, Option<Arc<Barrier>>) {
         // 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);
+                self.configuration
-            None
+                    .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<BarReprogrammingParams> {
-        self.configuration.detect_bar_reprogramming(reg_idx, data)
-    }
-
     fn allocate_bars(
         &mut self,
         _allocator: &Arc<Mutex<SystemAllocator>>,