Skip to content

Commit c4be916

Browse files
committed
refactor(pci): move (de)allocation BAR methods out of PciDevice
PciDevice trait was defining (optional) methods to allocate and deallocate BAR regions for devices. This logic is quite device specific and when invoked we typically know what device it refers to exactly. Currently, we only implement the allocate API for VirtIO devices (no deallocation whatsoever). Simplify things by dropping these APIs from the trait definition. Also, remove the 32bit allocator argument from allocate_bars(); we always only allocate a single 64-bit BAR for VirtIO devices. Signed-off-by: Babis Chalios <bchalios@amazon.es>
1 parent 4a21dc6 commit c4be916

File tree

3 files changed

+43
-69
lines changed

3 files changed

+43
-69
lines changed

src/pci/src/device.rs

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@
88
use std::sync::{Arc, Barrier};
99
use std::{io, result};
1010

11-
use vm_allocator::AddressAllocator;
12-
1311
use crate::configuration::{self, PciBarRegionType};
1412

1513
#[derive(Debug, thiserror::Error, displaydoc::Display)]
@@ -23,7 +21,6 @@ pub enum Error {
2321
/// Expected resource not found.
2422
MissingResource,
2523
}
26-
pub type Result<T> = std::result::Result<T, Error>;
2724

2825
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
2926
pub struct BarReprogrammingParams {
@@ -34,25 +31,6 @@ pub struct BarReprogrammingParams {
3431
}
3532

3633
pub trait PciDevice: Send {
37-
/// Allocates the needed PCI BARs space using the `allocate` function which takes a size and
38-
/// returns an address. Returns a Vec of (GuestAddress, GuestUsize) tuples.
39-
fn allocate_bars(
40-
&mut self,
41-
_mmio32_allocator: &mut AddressAllocator,
42-
_mmio64_allocator: &mut AddressAllocator,
43-
) -> Result<()> {
44-
Ok(())
45-
}
46-
47-
/// Frees the PCI BARs previously allocated with a call to allocate_bars().
48-
fn free_bars(
49-
&mut self,
50-
_mmio32_allocator: &mut AddressAllocator,
51-
_mmio64_allocator: &mut AddressAllocator,
52-
) -> Result<()> {
53-
Ok(())
54-
}
55-
5634
/// Sets a register in the configuration space.
5735
/// * `reg_idx` - The index of the config register to modify.
5836
/// * `offset` - Offset into the register.

src/vmm/src/device_manager/pci_mngr.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use std::sync::{Arc, Mutex};
88

99
use event_manager::{MutEventSubscriber, SubscriberOps};
1010
use log::{debug, error, warn};
11-
use pci::{PciBarRegionType, PciDevice, PciDeviceError, PciRootError};
11+
use pci::{PciBarRegionType, PciDeviceError, PciRootError};
1212
use serde::{Deserialize, Serialize};
1313
use vm_device::BusError;
1414

@@ -130,10 +130,7 @@ impl PciDevices {
130130
let mut resource_allocator_lock = vm.resource_allocator();
131131
let resource_allocator = resource_allocator_lock.deref_mut();
132132

133-
virtio_device.allocate_bars(
134-
&mut resource_allocator.mmio32_memory,
135-
&mut resource_allocator.mmio64_memory,
136-
)?;
133+
virtio_device.allocate_bars(&mut resource_allocator.mmio64_memory)?;
137134

138135
let virtio_device = Arc::new(Mutex::new(virtio_device));
139136
pci_segment

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

Lines changed: 41 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,47 @@ impl VirtioPciDevice {
359359
Ok(msix_config)
360360
}
361361

362+
pub fn allocate_bars(
363+
&mut self,
364+
mmio64_allocator: &mut AddressAllocator,
365+
) -> std::result::Result<(), PciDeviceError> {
366+
let device_clone = self.device.clone();
367+
let device = device_clone.lock().unwrap();
368+
369+
// Allocate the virtio-pci capability BAR.
370+
// See http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-740004
371+
let virtio_pci_bar_addr = mmio64_allocator
372+
.allocate(
373+
CAPABILITY_BAR_SIZE,
374+
CAPABILITY_BAR_SIZE,
375+
AllocPolicy::FirstMatch,
376+
)
377+
.unwrap()
378+
.start();
379+
380+
let bar = PciBarConfiguration {
381+
addr: virtio_pci_bar_addr,
382+
size: CAPABILITY_BAR_SIZE,
383+
idx: VIRTIO_COMMON_BAR_INDEX,
384+
region_type: PciBarRegionType::Memory64BitRegion,
385+
prefetchable: pci::PciBarPrefetchable::NotPrefetchable,
386+
};
387+
388+
// The creation of the PCI BAR and its associated capabilities must
389+
// happen only during the creation of a brand new VM. When a VM is
390+
// restored from a known state, the BARs are already created with the
391+
// right content, therefore we don't need to go through this codepath.
392+
self.configuration
393+
.add_pci_bar(&bar)
394+
.map_err(|e| PciDeviceError::IoRegistrationFailed(virtio_pci_bar_addr, e))?;
395+
396+
// Once the BARs are allocated, the capabilities can be added to the PCI configuration.
397+
self.add_pci_capabilities()?;
398+
self.bar_region = bar;
399+
400+
Ok(())
401+
}
402+
362403
/// Constructs a new PCI transport for the given virtio device.
363404
pub fn new(
364405
id: String,
@@ -798,48 +839,6 @@ impl PciDevice for VirtioPciDevice {
798839
self.configuration.detect_bar_reprogramming(reg_idx, data)
799840
}
800841

801-
fn allocate_bars(
802-
&mut self,
803-
mmio32_allocator: &mut AddressAllocator,
804-
mmio64_allocator: &mut AddressAllocator,
805-
) -> std::result::Result<(), PciDeviceError> {
806-
let device_clone = self.device.clone();
807-
let device = device_clone.lock().unwrap();
808-
809-
// Allocate the virtio-pci capability BAR.
810-
// See http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-740004
811-
let virtio_pci_bar_addr = mmio64_allocator
812-
.allocate(
813-
CAPABILITY_BAR_SIZE,
814-
CAPABILITY_BAR_SIZE,
815-
AllocPolicy::FirstMatch,
816-
)
817-
.unwrap()
818-
.start();
819-
820-
let bar = PciBarConfiguration {
821-
addr: virtio_pci_bar_addr,
822-
size: CAPABILITY_BAR_SIZE,
823-
idx: VIRTIO_COMMON_BAR_INDEX,
824-
region_type: PciBarRegionType::Memory64BitRegion,
825-
prefetchable: pci::PciBarPrefetchable::NotPrefetchable,
826-
};
827-
828-
// The creation of the PCI BAR and its associated capabilities must
829-
// happen only during the creation of a brand new VM. When a VM is
830-
// restored from a known state, the BARs are already created with the
831-
// right content, therefore we don't need to go through this codepath.
832-
self.configuration
833-
.add_pci_bar(&bar)
834-
.map_err(|e| PciDeviceError::IoRegistrationFailed(virtio_pci_bar_addr, e))?;
835-
836-
// Once the BARs are allocated, the capabilities can be added to the PCI configuration.
837-
self.add_pci_capabilities()?;
838-
self.bar_region = bar;
839-
840-
Ok(())
841-
}
842-
843842
fn move_bar(
844843
&mut self,
845844
old_base: u64,

0 commit comments

Comments
 (0)