pci: virtio: refactor: separate BARs logic from PciConfiguration

Currently logic for handling BARs is integrated into the
PciConfiguration type. This makes it impossible to reuse for
future VFIO work. In order to fix this, create a separate type
just for handling of BARs region and make VirtioPciDevice type
(the only user of PciConfiguration) to use that instead of
PciConfiguration. This makes the original PciConfiguration not
aware of any BARs which is fine. Future commits will remove BAR
handling from PciConfiguration type, so the duplication is temporary.

Signed-off-by: Egor Lazarchuk <[email protected]>

Author: ShadowCurse

src/vmm/src/device_manager/pci_mngr.rs

@@ -93,11 +93,11 @@ impl PciDevices {
 
         debug!(
             "Inserting MMIO BAR region: {:#x}:{:#x}",
-            virtio_device_locked.bar_address, CAPABILITY_BAR_SIZE
+            virtio_device_locked.config_bar_addr(), CAPABILITY_BAR_SIZE
         );
         vm.common.mmio_bus.insert(
             virtio_device.clone(),
-            virtio_device_locked.bar_address,
+            virtio_device_locked.config_bar_addr(),
             CAPABILITY_BAR_SIZE,
         )?;
 

src/vmm/src/devices/virtio/transport/pci/device.rs

@@ -26,6 +26,7 @@ use vm_allocator::{AddressAllocator, AllocPolicy, RangeInclusive};
 use vm_memory::{Address, ByteValued, GuestAddress, Le32};
 use vmm_sys_util::errno;
 use vmm_sys_util::eventfd::EventFd;
+use zerocopy::IntoBytes;
 
 use crate::Vm;
 use crate::devices::virtio::device::{VirtioDevice, VirtioDeviceType};
@@ -36,7 +37,9 @@ use crate::devices::virtio::transport::pci::common_config::{
 };
 use crate::devices::virtio::transport::{VirtioInterrupt, VirtioInterruptType};
 use crate::logger::{debug, error};
-use crate::pci::configuration::{PciCapability, PciConfiguration, PciConfigurationState};
+use crate::pci::configuration::{
+    BAR0_REG, Bars, NUM_BAR_REGS, PciCapability, PciConfiguration, PciConfigurationState,
+};
 use crate::pci::msix::{MsixCap, MsixConfig, MsixConfigState};
 use crate::pci::{BarReprogrammingParams, DeviceRelocationError, PciDevice};
 use crate::snapshot::Persist;
@@ -242,7 +245,7 @@ pub struct VirtioPciDeviceState {
     pub pci_configuration_state: PciConfigurationState,
     pub pci_dev_state: VirtioPciCommonConfigState,
     pub msix_state: MsixConfigState,
-    pub bar_address: u64,
+    pub bars: Bars,
 }
 
 #[derive(Debug, thiserror::Error, displaydoc::Display)]
@@ -286,8 +289,8 @@ pub struct VirtioPciDevice {
     // a device.
     cap_pci_cfg_info: VirtioPciCfgCapInfo,
 
-    // Allocated address for the BAR
-    pub bar_address: u64,
+    // BARs region for the device
+    pub bars: Bars,
 }
 
 impl Debug for VirtioPciDevice {
@@ -350,16 +353,14 @@ impl VirtioPciDevice {
             )
             .unwrap()
             .start();
-
-        self.configuration.add_pci_bar(
-            VIRTIO_BAR_INDEX as usize,
+        // Virtio BAR is 64bit BAR without prefetchable bit since it is an MMIO region.
+        self.bars.set_bar_64(
+            VIRTIO_BAR_INDEX,
             virtio_pci_bar_addr,
             CAPABILITY_BAR_SIZE,
+            false,
         );
-
-        // Once the BARs are allocated, the capabilities can be added to the PCI configuration.
         self.add_pci_capabilities();
-        self.bar_address = virtio_pci_bar_addr;
     }
 
     /// Constructs a new PCI transport for the given virtio device.
@@ -408,7 +409,7 @@ impl VirtioPciDevice {
             virtio_interrupt: Some(interrupt),
             memory,
             cap_pci_cfg_info: VirtioPciCfgCapInfo::default(),
-            bar_address: 0,
+            bars: Bars::default(),
         };
 
         Ok(virtio_pci_device)
@@ -453,7 +454,7 @@ impl VirtioPciDevice {
             virtio_interrupt: Some(interrupt),
             memory: vm.guest_memory().clone(),
             cap_pci_cfg_info,
-            bar_address: state.bar_address,
+            bars: state.bars,
         };
 
         if state.device_activated {
@@ -483,7 +484,7 @@ impl VirtioPciDevice {
     }
 
     pub fn config_bar_addr(&self) -> u64 {
-        self.configuration.get_bar_addr(VIRTIO_BAR_INDEX as usize)
+        self.bars.get_bar_addr_64(VIRTIO_BAR_INDEX)
     }
 
     fn add_pci_capabilities(&mut self) {
@@ -638,7 +639,7 @@ impl VirtioPciDevice {
                 .lock()
                 .expect("Poisoned lock")
                 .state(),
-            bar_address: self.bar_address,
+            bars: self.bars,
         }
     }
 }
@@ -742,42 +743,63 @@ impl PciDevice for VirtioPciDevice {
         offset: u64,
         data: &[u8],
     ) -> 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.
-        let base = reg_idx * 4;
-        if base + u64_to_usize(offset) >= self.cap_pci_cfg_info.offset
-            && base + u64_to_usize(offset) + data.len()
-                <= self.cap_pci_cfg_info.offset + self.cap_pci_cfg_info.cap.bytes().len()
-        {
-            let offset = base + u64_to_usize(offset) - self.cap_pci_cfg_info.offset;
-            self.write_cap_pci_cfg(offset, data)
-        } else {
-            self.configuration
-                .write_config_register(reg_idx, offset, data);
+        if BAR0_REG as usize <= reg_idx && reg_idx < (BAR0_REG + NUM_BAR_REGS) as usize {
+            // reg_idx is in [BAR0_REG, BAR0_REG+NUM_BAR_REGS), so the difference is 0..5.
+            #[allow(clippy::cast_possible_truncation)]
+            let bar_idx = (reg_idx - BAR0_REG as usize) as u8;
+            // offset is within a 4-byte PCI config register (0..3).
+            #[allow(clippy::cast_possible_truncation)]
+            let offset = offset as u8;
+            self.bars.write(bar_idx, offset, data);
             None
+        } else {
+            // 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.
+            let base = reg_idx * 4;
+            if base + u64_to_usize(offset) >= self.cap_pci_cfg_info.offset
+                && base + u64_to_usize(offset) + data.len()
+                    <= self.cap_pci_cfg_info.offset + self.cap_pci_cfg_info.cap.bytes().len()
+            {
+                let offset = base + u64_to_usize(offset) - self.cap_pci_cfg_info.offset;
+                self.write_cap_pci_cfg(offset, data)
+            } else {
+                self.configuration
+                    .write_config_register(reg_idx, offset, data);
+                None
+            }
         }
     }
 
     fn read_config_register(&mut self, reg_idx: usize) -> u32 {
-        // 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.
-        let base = reg_idx * 4;
-        if base >= self.cap_pci_cfg_info.offset
-            && base + 4 <= self.cap_pci_cfg_info.offset + self.cap_pci_cfg_info.cap.bytes().len()
-        {
-            let offset = base - self.cap_pci_cfg_info.offset;
-            let mut data = [0u8; 4];
-            let len = u32::from(self.cap_pci_cfg_info.cap.cap.length) as usize;
-            if len <= 4 {
-                self.read_cap_pci_cfg(offset, &mut data[..len]);
-                u32::from_le_bytes(data)
+        if BAR0_REG as usize <= reg_idx && reg_idx < (BAR0_REG + NUM_BAR_REGS) as usize {
+            // reg_idx is in [BAR0_REG, BAR0_REG+NUM_BAR_REGS), so the difference is 0..5.
+            #[allow(clippy::cast_possible_truncation)]
+            let bar_idx = (reg_idx - BAR0_REG as usize) as u8;
+            let mut value: u32 = 0;
+            self.bars.read(bar_idx, 0, value.as_mut_bytes());
+            value
+        } else {
+            // 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.
+            let base = reg_idx * 4;
+            if base >= self.cap_pci_cfg_info.offset
+                && base + 4
+                    <= self.cap_pci_cfg_info.offset + self.cap_pci_cfg_info.cap.bytes().len()
+            {
+                let offset = base - self.cap_pci_cfg_info.offset;
+                let mut data = [0u8; 4];
+                let len = u32::from(self.cap_pci_cfg_info.cap.cap.length) as usize;
+                if len <= 4 {
+                    self.read_cap_pci_cfg(offset, &mut data[..len]);
+                    u32::from_le_bytes(data)
+                } else {
+                    0
+                }
             } else {
-                0
+                self.configuration.read_reg(reg_idx)
             }
-        } else {
-            self.configuration.read_reg(reg_idx)
         }
     }
 
@@ -786,7 +808,7 @@ impl PciDevice for VirtioPciDevice {
         reg_idx: usize,
         data: &[u8],
     ) -> Option<BarReprogrammingParams> {
-        self.configuration.detect_bar_reprogramming(reg_idx, data)
+        None
     }
 
     fn read_bar(&mut self, _base: u64, offset: u64, data: &mut [u8]) {