Skip to content

Commit 7194977

Browse files
committed
refactor: remove unused bar relocation related items
Since we don't support BAR relocation we don't need all of this machinery right now. It was lifted with original PCIe code from Cloud-Hypervisor but never got used. Now with recent commits it became even more useless. Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
1 parent 4e2673b commit 7194977

5 files changed

Lines changed: 19 additions & 149 deletions

File tree

src/vmm/src/devices/pci/pci_segment.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ impl std::fmt::Debug for PciSegment {
7171
impl PciSegment {
7272
fn build(id: u16, vm: &Arc<Vm>, pci_irq_slots: &[u8; 32]) -> Result<PciSegment, BusError> {
7373
let pci_root = PciRoot::new(None);
74-
let pci_bus = Arc::new(Mutex::new(PciBus::new(pci_root, vm.clone())));
74+
let pci_bus = Arc::new(Mutex::new(PciBus::new(pci_root)));
7575

7676
let pci_config_mmio = Arc::new(Mutex::new(PciConfigMmio::new(Arc::clone(&pci_bus))));
7777
let mmio_config_address = PCI_MMCONFIG_START + PCI_MMIO_CONFIG_SIZE_PER_SEGMENT * id as u64;

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

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ use crate::pci::configuration::{
3939
};
4040
use crate::pci::msix::{MsixCap, MsixConfig, MsixConfigState};
4141
use crate::pci::{
42-
BarReprogrammingParams, DeviceRelocationError, PciCapabilityId, PciClassCode, PciDevice,
43-
PciMassStorageSubclass, PciNetworkControllerSubclass, PciSBDF,
42+
BarReprogrammingParams, PciCapabilityId, PciClassCode, PciDevice, PciMassStorageSubclass,
43+
PciNetworkControllerSubclass, PciSBDF,
4444
};
4545
use crate::snapshot::Persist;
4646
use crate::vstate::bus::BusDevice;
@@ -249,8 +249,6 @@ pub struct VirtioPciDeviceState {
249249

250250
#[derive(Debug, thiserror::Error, displaydoc::Display)]
251251
pub enum VirtioPciDeviceError {
252-
/// Failed creating VirtioPciDevice: {0}
253-
CreateVirtioPciDevice(#[from] DeviceRelocationError),
254252
/// Error creating MSI configuration: {0}
255253
Msi(#[from] InterruptError),
256254
/// Invalid PCI configuration state: {0}
@@ -804,18 +802,6 @@ impl PciDevice for VirtioPciDevice {
804802
}
805803
}
806804

807-
fn detect_bar_reprogramming(
808-
&mut self,
809-
reg_idx: u16,
810-
data: &[u8],
811-
) -> Option<BarReprogrammingParams> {
812-
None
813-
}
814-
815-
fn move_bar(&mut self, old_base: u64, new_base: u64) -> Result<(), DeviceRelocationError> {
816-
Ok(())
817-
}
818-
819805
fn read_bar(&mut self, _base: u64, offset: u64, data: &mut [u8]) {
820806
match offset {
821807
o if o < u64::from(COMMON_CONFIG_BAR_OFFSET + COMMON_CONFIG_SIZE) => {

src/vmm/src/pci/bus.rs

Lines changed: 16 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,12 @@
77

88
use std::collections::HashMap;
99
use std::fmt::Debug;
10-
use std::ops::DerefMut;
1110
use std::sync::{Arc, Barrier, Mutex};
1211

1312
use byteorder::{ByteOrder, LittleEndian};
1413

15-
use crate::logger::error;
1614
use crate::pci::configuration::PciConfiguration;
17-
use crate::pci::{DeviceRelocation, PciBridgeSubclass, PciClassCode, PciDevice};
15+
use crate::pci::{PciBridgeSubclass, PciClassCode, PciDevice};
1816
use crate::utils::u64_to_usize;
1917
use crate::vstate::bus::BusDevice;
2018

@@ -80,7 +78,6 @@ pub struct PciBus {
8078
/// Devices attached to this bus.
8179
/// Device 0 is host bridge.
8280
pub devices: HashMap<u8, Arc<Mutex<dyn PciDevice>>>,
83-
vm: Arc<dyn DeviceRelocation>,
8481
device_ids: Vec<bool>,
8582
}
8683

@@ -94,7 +91,7 @@ impl Debug for PciBus {
9491

9592
impl PciBus {
9693
/// Create a new PCI bus
97-
pub fn new(pci_root: PciRoot, vm: Arc<dyn DeviceRelocation>) -> Self {
94+
pub fn new(pci_root: PciRoot) -> Self {
9895
let mut devices: HashMap<u8, Arc<Mutex<dyn PciDevice>>> = HashMap::new();
9996
let mut device_ids: Vec<bool> = vec![false; NUM_DEVICE_IDS];
10097

@@ -103,7 +100,6 @@ impl PciBus {
103100

104101
PciBus {
105102
devices,
106-
vm,
107103
device_ids,
108104
}
109105
}
@@ -220,22 +216,6 @@ impl PciConfigIo {
220216
if let Some(d) = pci_bus.devices.get(&device) {
221217
let mut device = d.lock().unwrap();
222218

223-
// Find out if one of the device's BAR is being reprogrammed, and
224-
// reprogram it if needed.
225-
if let Some(params) = device.detect_bar_reprogramming(register, data)
226-
&& let Err(e) = pci_bus.vm.move_bar(
227-
params.old_base,
228-
params.new_base,
229-
params.len,
230-
device.deref_mut(),
231-
)
232-
{
233-
error!(
234-
"Failed moving device BAR: {}: 0x{:x}->0x{:x}(0x{:x})",
235-
e, params.old_base, params.new_base, params.len
236-
);
237-
}
238-
239219
// offset is validated to be < 4 at the top of this function.
240220
#[allow(clippy::cast_possible_truncation)]
241221
let offset = offset as u8;
@@ -360,22 +340,6 @@ impl PciConfigMmio {
360340
if let Some(d) = pci_bus.devices.get(&device) {
361341
let mut device = d.lock().unwrap();
362342

363-
// Find out if one of the device's BAR is being reprogrammed, and
364-
// reprogram it if needed.
365-
if let Some(params) = device.detect_bar_reprogramming(register, data)
366-
&& let Err(e) = pci_bus.vm.move_bar(
367-
params.old_base,
368-
params.new_base,
369-
params.len,
370-
device.deref_mut(),
371-
)
372-
{
373-
error!(
374-
"Failed moving device BAR: {}: 0x{:x}->0x{:x}(0x{:x})",
375-
e, params.old_base, params.new_base, params.len
376-
);
377-
}
378-
379343
// offset is validated to be < 4 at the top of this function.
380344
#[allow(clippy::cast_possible_truncation)]
381345
let offset = offset as u8;
@@ -474,37 +438,14 @@ fn parse_io_config_address(config_address: u32) -> (u8, u8, u8, u16) {
474438

475439
#[cfg(test)]
476440
mod tests {
477-
use std::sync::atomic::AtomicUsize;
478441
use std::sync::{Arc, Mutex};
479442

480443
use super::{PciBus, PciConfigIo, PciConfigMmio, PciRoot};
481444
use crate::pci::bus::{DEVICE_ID_INTEL_VIRT_PCIE_HOST, VENDOR_ID_INTEL};
482445
use crate::pci::configuration::PciConfiguration;
483-
use crate::pci::{
484-
BarReprogrammingParams, DeviceRelocation, DeviceRelocationError, PciClassCode, PciDevice,
485-
PciMassStorageSubclass,
486-
};
446+
use crate::pci::{PciClassCode, PciDevice, PciMassStorageSubclass};
487447
use crate::vstate::bus::BusDevice;
488448

489-
#[derive(Debug, Default)]
490-
struct RelocationMock {
491-
reloc_cnt: AtomicUsize,
492-
}
493-
494-
impl DeviceRelocation for RelocationMock {
495-
fn move_bar(
496-
&self,
497-
_old_base: u64,
498-
_new_base: u64,
499-
_len: u64,
500-
_pci_dev: &mut dyn PciDevice,
501-
) -> Result<(), DeviceRelocationError> {
502-
self.reloc_cnt
503-
.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
504-
Ok(())
505-
}
506-
}
507-
508449
struct PciDevMock(PciConfiguration);
509450

510451
impl PciDevMock {
@@ -536,21 +477,12 @@ mod tests {
536477
fn read_config_register(&mut self, reg_idx: u16) -> u32 {
537478
self.0.read_reg(reg_idx)
538479
}
539-
540-
fn detect_bar_reprogramming(
541-
&mut self,
542-
_reg_idx: u16,
543-
_data: &[u8],
544-
) -> Option<BarReprogrammingParams> {
545-
None
546-
}
547480
}
548481

549482
#[test]
550483
fn test_writing_io_config_address() {
551-
let mock = Arc::new(RelocationMock::default());
552484
let root = PciRoot::new(None);
553-
let mut bus = PciConfigIo::new(Arc::new(Mutex::new(PciBus::new(root, mock))));
485+
let mut bus = PciConfigIo::new(Arc::new(Mutex::new(PciBus::new(root))));
554486

555487
assert_eq!(bus.config_address, 0);
556488
// Writing more than 32 bits will should fail
@@ -590,9 +522,8 @@ mod tests {
590522

591523
#[test]
592524
fn test_reading_io_config_address() {
593-
let mock = Arc::new(RelocationMock::default());
594525
let root = PciRoot::new(None);
595-
let mut bus = PciConfigIo::new(Arc::new(Mutex::new(PciBus::new(root, mock))));
526+
let mut bus = PciConfigIo::new(Arc::new(Mutex::new(PciBus::new(root))));
596527

597528
let mut buffer = [0u8; 4];
598529

@@ -637,19 +568,18 @@ mod tests {
637568
assert_eq!(buffer, [0x45, 0x44, 0x43, 0x42]);
638569
}
639570

640-
fn initialize_bus() -> (PciConfigMmio, PciConfigIo, Arc<RelocationMock>) {
641-
let mock = Arc::new(RelocationMock::default());
571+
fn initialize_bus() -> (PciConfigMmio, PciConfigIo) {
642572
let root = PciRoot::new(None);
643-
let mut bus = PciBus::new(root, mock.clone());
573+
let mut bus = PciBus::new(root);
644574
bus.add_device(1, Arc::new(Mutex::new(PciDevMock::new())));
645575

646576
let bus = Arc::new(Mutex::new(bus));
647-
(PciConfigMmio::new(bus.clone()), PciConfigIo::new(bus), mock)
577+
(PciConfigMmio::new(bus.clone()), PciConfigIo::new(bus))
648578
}
649579

650580
#[test]
651581
fn test_invalid_register_boundary_reads() {
652-
let (mut mmio_config, mut io_config, _) = initialize_bus();
582+
let (mut mmio_config, mut io_config) = initialize_bus();
653583

654584
// Read crossing register boundaries
655585
let mut buffer = [0u8; 4];
@@ -784,7 +714,7 @@ mod tests {
784714

785715
#[test]
786716
fn test_mmio_invalid_bus_number() {
787-
let (mut mmio_config, _, _) = initialize_bus();
717+
let (mut mmio_config, _) = initialize_bus();
788718
let mut buffer = [0u8; 4];
789719

790720
// Asking for Bus 1 should return all 1s
@@ -809,7 +739,7 @@ mod tests {
809739

810740
#[test]
811741
fn test_io_invalid_bus_number() {
812-
let (_, mut pio_config, _) = initialize_bus();
742+
let (_, mut pio_config) = initialize_bus();
813743
let mut buffer = [0u8; 4];
814744

815745
// Asking for Bus 1 should return all 1s
@@ -827,7 +757,7 @@ mod tests {
827757

828758
#[test]
829759
fn test_mmio_invalid_function() {
830-
let (mut mmio_config, _, _) = initialize_bus();
760+
let (mut mmio_config, _) = initialize_bus();
831761
let mut buffer = [0u8; 4];
832762

833763
// Asking for Bus 1 should return all 1s
@@ -852,7 +782,7 @@ mod tests {
852782

853783
#[test]
854784
fn test_io_invalid_function() {
855-
let (_, mut pio_config, _) = initialize_bus();
785+
let (_, mut pio_config) = initialize_bus();
856786
let mut buffer = [0u8; 4];
857787

858788
// Asking for Bus 1 should return all 1s
@@ -870,7 +800,7 @@ mod tests {
870800

871801
#[test]
872802
fn test_io_disabled_reads() {
873-
let (_, mut pio_config, _) = initialize_bus();
803+
let (_, mut pio_config) = initialize_bus();
874804
let mut buffer = [0u8; 4];
875805

876806
// Trying to read without enabling should return all 1s
@@ -888,7 +818,7 @@ mod tests {
888818

889819
#[test]
890820
fn test_io_disabled_writes() {
891-
let (_, mut pio_config, _) = initialize_bus();
821+
let (_, mut pio_config) = initialize_bus();
892822

893823
// Try to write the IRQ line used for the root port.
894824
let mut buffer = [0u8; 4];
@@ -916,7 +846,7 @@ mod tests {
916846

917847
#[test]
918848
fn test_mmio_writes() {
919-
let (mut mmio_config, _, _) = initialize_bus();
849+
let (mut mmio_config, _) = initialize_bus();
920850
let mut buffer = [0u8; 4];
921851

922852
read_mmio_config(&mut mmio_config, 0, 0, 0, 15, 0, &mut buffer);

src/vmm/src/pci/mod.rs

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -42,14 +42,6 @@ pub trait PciDevice: Send {
4242
/// Gets a register from the configuration space.
4343
/// * `reg_idx` - The index of the config register to read.
4444
fn read_config_register(&mut self, reg_idx: u16) -> u32;
45-
/// Detects if a BAR is being reprogrammed.
46-
fn detect_bar_reprogramming(
47-
&mut self,
48-
_reg_idx: u16,
49-
_data: &[u8],
50-
) -> Option<BarReprogrammingParams> {
51-
None
52-
}
5345
/// Reads from a BAR region mapped into the device.
5446
/// * `addr` - The guest address inside the BAR.
5547
/// * `data` - Filled with the data from `addr`.
@@ -60,31 +52,6 @@ pub trait PciDevice: Send {
6052
fn write_bar(&mut self, _base: u64, _offset: u64, _data: &[u8]) -> Option<Arc<Barrier>> {
6153
None
6254
}
63-
/// Relocates the BAR to a different address in guest address space.
64-
fn move_bar(&mut self, _old_base: u64, _new_base: u64) -> Result<(), DeviceRelocationError> {
65-
Ok(())
66-
}
67-
}
68-
69-
/// Errors for device manager.
70-
#[derive(Debug, thiserror::Error, displaydoc::Display)]
71-
pub enum DeviceRelocationError {
72-
/// Device relocation not supported.
73-
NotSupported,
74-
}
75-
76-
/// This trait defines a set of functions which can be triggered whenever a
77-
/// PCI device is modified in any way.
78-
pub trait DeviceRelocation: Send + Sync {
79-
/// The BAR needs to be moved to a different location in the guest address
80-
/// space. This follows a decision from the software running in the guest.
81-
fn move_bar(
82-
&self,
83-
old_base: u64,
84-
new_base: u64,
85-
len: u64,
86-
pci_dev: &mut dyn PciDevice,
87-
) -> Result<(), DeviceRelocationError>;
8855
}
8956

9057
/// Segment with associated Device BDF

src/vmm/src/vstate/vm.rs

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ use vmm_sys_util::eventfd::EventFd;
2626
pub use crate::arch::{ArchVm as Vm, ArchVmError, VmState};
2727
use crate::arch::{GSI_MSI_END, host_page_size};
2828
use crate::logger::{debug, info};
29-
use crate::pci::{DeviceRelocation, DeviceRelocationError, PciDevice};
3029
use crate::persist::CreateSnapshotError;
3130
use crate::vmm_config::snapshot::SnapshotType;
3231
use crate::vstate::bus::Bus;
@@ -522,18 +521,6 @@ fn mincore_bitmap(addr: *mut u8, len: usize) -> Result<Vec<u64>, VmError> {
522521
Ok(bitmap)
523522
}
524523

525-
impl DeviceRelocation for Vm {
526-
fn move_bar(
527-
&self,
528-
_old_base: u64,
529-
_new_base: u64,
530-
_len: u64,
531-
_pci_dev: &mut dyn PciDevice,
532-
) -> Result<(), DeviceRelocationError> {
533-
Err(DeviceRelocationError::NotSupported)
534-
}
535-
}
536-
537524
#[cfg(test)]
538525
pub(crate) mod tests {
539526
use std::sync::atomic::Ordering;

0 commit comments

Comments
 (0)