Skip to content

Minor cleanup in PCI code #5342

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 4, 2025
Merged

Minor cleanup in PCI code #5342

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4c404e1
pci: remove unused code
bchalios Aug 1, 2025
1719869
pci: remove unused type from PciDevice::allocate_bars
bchalios Aug 1, 2025
d8b8d1f
pci: always assume we are using a single 64bit MMIO BAR
bchalios Aug 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 2 additions & 56 deletions src/pci/src/bus.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,10 @@ use std::ops::DerefMut;
use std::sync::{Arc, Barrier, Mutex};

use byteorder::{ByteOrder, LittleEndian};
use vm_device::{Bus, BusDevice, BusDeviceSync};
use vm_device::BusDevice;

use crate::configuration::{
PciBarRegionType, PciBridgeSubclass, PciClassCode, PciConfiguration, PciHeaderType,
};
use crate::configuration::{PciBridgeSubclass, PciClassCode, PciConfiguration, PciHeaderType};
use crate::device::{DeviceRelocation, Error as PciDeviceError, PciDevice};
use crate::PciBarConfiguration;

const VENDOR_ID_INTEL: u16 = 0x8086;
const DEVICE_ID_INTEL_VIRT_PCIE_HOST: u16 = 0x0d57;
Expand Down Expand Up @@ -123,40 +120,11 @@ impl PciBus {
}
}

pub fn register_mapping(
&self,
dev: Arc<dyn BusDeviceSync>,
io_bus: &Bus,
mmio_bus: &Bus,
bars: Vec<PciBarConfiguration>,
) -> Result<()> {
for bar in bars {
match bar.region_type() {
PciBarRegionType::IoRegion => {
io_bus
.insert(dev.clone(), bar.addr(), bar.size())
.map_err(PciRootError::PioInsert)?;
}
PciBarRegionType::Memory32BitRegion | PciBarRegionType::Memory64BitRegion => {
mmio_bus
.insert(dev.clone(), bar.addr(), bar.size())
.map_err(PciRootError::MmioInsert)?;
}
}
}
Ok(())
}

pub fn add_device(&mut self, device_id: u32, device: Arc<Mutex<dyn PciDevice>>) -> Result<()> {
self.devices.insert(device_id, device);
Ok(())
}

pub fn remove_by_device(&mut self, device: &Arc<Mutex<dyn PciDevice>>) -> Result<()> {
self.devices.retain(|_, dev| !Arc::ptr_eq(dev, device));
Ok(())
}

pub fn next_device_id(&mut self) -> Result<u32> {
for (idx, device_id) in self.device_ids.iter_mut().enumerate() {
if !(*device_id) {
Expand All @@ -167,28 +135,6 @@ impl PciBus {

Err(PciRootError::NoPciDeviceSlotAvailable)
}

pub fn get_device_id(&mut self, id: usize) -> Result<()> {
if id < NUM_DEVICE_IDS {
if !self.device_ids[id] {
self.device_ids[id] = true;
Ok(())
} else {
Err(PciRootError::AlreadyInUsePciDeviceSlot(id))
}
} else {
Err(PciRootError::InvalidPciDeviceSlot(id))
}
}

pub fn put_device_id(&mut self, id: usize) -> Result<()> {
if id < NUM_DEVICE_IDS {
self.device_ids[id] = false;
Ok(())
} else {
Err(PciRootError::InvalidPciDeviceSlot(id))
}
}
}

pub struct PciConfigIo {
Expand Down
153 changes: 6 additions & 147 deletions src/pci/src/configuration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,9 @@ use std::sync::{Arc, Mutex};

use byteorder::{ByteOrder, LittleEndian};
use serde::{Deserialize, Serialize};
use vm_device::PciBarType;

use crate::device::BarReprogrammingParams;
use crate::{MsixConfig, PciInterruptPin};
use crate::MsixConfig;

// The number of 32bit registers in the config space, 4096 bytes.
const NUM_CONFIGURATION_REGISTERS: usize = 1024;
Expand All @@ -22,7 +21,6 @@ const STATUS_REG: usize = 1;
const STATUS_REG_CAPABILITIES_USED_MASK: u32 = 0x0010_0000;
const BAR0_REG: usize = 4;
const ROM_BAR_REG: usize = 12;
const ROM_BAR_IDX: usize = 6;
const BAR_IO_ADDR_MASK: u32 = 0xffff_fffc;
const BAR_MEM_ADDR_MASK: u32 = 0xffff_fff0;
const ROM_BAR_ADDR_MASK: u32 = 0xffff_f800;
Expand All @@ -33,8 +31,6 @@ const CAPABILITY_LIST_HEAD_OFFSET: usize = 0x34;
const FIRST_CAPABILITY_OFFSET: usize = 0x40;
const CAPABILITY_MAX_OFFSET: usize = 192;

const INTERRUPT_LINE_PIN_REG: usize = 15;

pub const PCI_CONFIGURATION_ID: &str = "pci_configuration";

/// Represents the types of PCI headers allowed in the configuration registers.
Expand Down Expand Up @@ -446,26 +442,6 @@ pub enum PciBarRegionType {
Memory64BitRegion = 0x04,
}

impl From<PciBarType> for PciBarRegionType {
fn from(type_: PciBarType) -> Self {
match type_ {
PciBarType::Io => PciBarRegionType::IoRegion,
PciBarType::Mmio32 => PciBarRegionType::Memory32BitRegion,
PciBarType::Mmio64 => PciBarRegionType::Memory64BitRegion,
}
}
}

impl From<PciBarRegionType> for PciBarType {
fn from(val: PciBarRegionType) -> Self {
match val {
PciBarRegionType::IoRegion => PciBarType::Io,
PciBarRegionType::Memory32BitRegion => PciBarType::Mmio32,
PciBarRegionType::Memory64BitRegion => PciBarType::Mmio64,
}
}
}

#[derive(Debug, Copy, Clone, Serialize, Deserialize)]
pub enum PciBarPrefetchable {
NotPrefetchable = 0,
Expand All @@ -483,11 +459,11 @@ impl From<PciBarPrefetchable> for bool {

#[derive(Debug, Copy, Clone, Serialize, Deserialize)]
pub struct PciBarConfiguration {
addr: u64,
size: u64,
idx: usize,
region_type: PciBarRegionType,
prefetchable: PciBarPrefetchable,
pub addr: u64,
pub size: u64,
pub idx: usize,
pub region_type: PciBarRegionType,
pub prefetchable: PciBarPrefetchable,
}

#[derive(Debug)]
Expand Down Expand Up @@ -797,42 +773,6 @@ impl PciConfiguration {
Ok(())
}

/// Adds rom expansion BAR.
pub fn add_pci_rom_bar(&mut self, config: &PciBarConfiguration, active: u32) -> Result<()> {
let bar_idx = config.idx;
let reg_idx = ROM_BAR_REG;

if self.rom_bar_used {
return Err(Error::RomBarInUse(bar_idx));
}

if !config.size.is_power_of_two() {
return Err(Error::RomBarSizeInvalid(config.size));
}

if bar_idx != ROM_BAR_IDX {
return Err(Error::RomBarInvalid(bar_idx));
}

let end_addr = config
.addr
.checked_add(config.size - 1)
.ok_or(Error::RomBarAddressInvalid(config.addr, config.size))?;

if end_addr > u64::from(u32::MAX) {
return Err(Error::RomBarAddressInvalid(config.addr, config.size));
}

self.registers[reg_idx] = (config.addr as u32) | active;
self.writable_bits[reg_idx] = ROM_BAR_ADDR_MASK;
self.rom_bar_addr = self.registers[reg_idx];
self.rom_bar_size =
encode_32_bits_bar_size(config.size as u32).ok_or(Error::Encode32BarSize)?;
self.rom_bar_used = true;

Ok(())
}

/// Returns the address of the given BAR region.
pub fn get_bar_addr(&self, bar_num: usize) -> u64 {
let bar_idx = BAR0_REG + bar_num;
Expand All @@ -848,16 +788,6 @@ impl PciConfiguration {
addr
}

/// Configures the IRQ line and pin used by this device.
pub fn set_irq(&mut self, line: u8, pin: PciInterruptPin) {
// `pin` is 1-based in the pci config space.
let pin_idx = (pin as u32) + 1;
self.registers[INTERRUPT_LINE_PIN_REG] = (self.registers[INTERRUPT_LINE_PIN_REG]
& 0xffff_0000)
| (pin_idx << 8)
| u32::from(line);
}

/// Adds the capability `cap_data` to the list of capabilities.
/// `cap_data` should include the two-byte PCI capability header (type, next),
/// but not populate it. Correct values will be generated automatically based
Expand Down Expand Up @@ -940,10 +870,6 @@ impl PciConfiguration {
}
}

pub fn read_config_register(&self, reg_idx: usize) -> u32 {
self.read_reg(reg_idx)
}

pub fn detect_bar_reprogramming(
&mut self,
reg_idx: usize,
Expand Down Expand Up @@ -1074,73 +1000,6 @@ impl Default for PciBarConfiguration {
}
}

impl PciBarConfiguration {
pub fn new(
idx: usize,
size: u64,
region_type: PciBarRegionType,
prefetchable: PciBarPrefetchable,
) -> Self {
PciBarConfiguration {
idx,
addr: 0,
size,
region_type,
prefetchable,
}
}

#[must_use]
pub fn set_index(mut self, idx: usize) -> Self {
self.idx = idx;
self
}

#[must_use]
pub fn set_address(mut self, addr: u64) -> Self {
self.addr = addr;
self
}

#[must_use]
pub fn set_size(mut self, size: u64) -> Self {
self.size = size;
self
}

#[must_use]
pub fn set_region_type(mut self, region_type: PciBarRegionType) -> Self {
self.region_type = region_type;
self
}

#[must_use]
pub fn set_prefetchable(mut self, prefetchable: PciBarPrefetchable) -> Self {
self.prefetchable = prefetchable;
self
}

pub fn idx(&self) -> usize {
self.idx
}

pub fn addr(&self) -> u64 {
self.addr
}

pub fn size(&self) -> u64 {
self.size
}

pub fn region_type(&self) -> PciBarRegionType {
self.region_type
}

pub fn prefetchable(&self) -> PciBarPrefetchable {
self.prefetchable
}
}

#[cfg(test)]
mod tests {
use vm_memory::ByteValued;
Expand Down
4 changes: 0 additions & 4 deletions src/pci/src/device.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use std::sync::{Arc, Barrier};
use std::{io, result};

use vm_allocator::AddressAllocator;
use vm_device::Resource;

use crate::configuration::{self, PciBarRegionType};
use crate::PciBarConfiguration;
Expand All @@ -25,8 +24,6 @@ pub enum Error {
IoRegistrationFailed(u64, configuration::Error),
/// Expected resource not found.
MissingResource,
/// Invalid resource
InvalidResource(Resource),
}
pub type Result<T> = std::result::Result<T, Error>;

Expand All @@ -45,7 +42,6 @@ pub trait PciDevice: Send {
&mut self,
_mmio32_allocator: &mut AddressAllocator,
_mmio64_allocator: &mut AddressAllocator,
_resources: Option<Vec<Resource>>,
) -> Result<Vec<PciBarConfiguration>> {
Ok(Vec::new())
}
Expand Down
35 changes: 8 additions & 27 deletions src/vmm/src/device_manager/pci_mngr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,32 +88,14 @@ impl PciDevices {
vm: &Vm,
virtio_device: &Arc<Mutex<VirtioPciDevice>>,
) -> Result<(), PciManagerError> {
for bar in &virtio_device.lock().expect("Poisoned lock").bar_regions {
match bar.region_type() {
PciBarRegionType::IoRegion => {
debug!(
"Inserting I/O BAR region: {:#x}:{:#x}",
bar.addr(),
bar.size()
);
#[cfg(target_arch = "x86_64")]
vm.pio_bus
.insert(virtio_device.clone(), bar.addr(), bar.size())?;
#[cfg(target_arch = "aarch64")]
log::error!("pci: We do not support I/O region allocation")
}
PciBarRegionType::Memory32BitRegion | PciBarRegionType::Memory64BitRegion => {
debug!(
"Inserting MMIO BAR region: {:#x}:{:#x}",
bar.addr(),
bar.size()
);
vm.common
.mmio_bus
.insert(virtio_device.clone(), bar.addr(), bar.size())?;
}
}
}
let virtio_device_locked = virtio_device.lock().expect("Poisoned lock");
let bar = &virtio_device_locked.bar_region;
assert_eq!(bar.region_type, PciBarRegionType::Memory64BitRegion);

debug!("Inserting MMIO BAR region: {:#x}:{:#x}", bar.addr, bar.size);
vm.common
.mmio_bus
.insert(virtio_device.clone(), bar.addr, bar.size)?;

Ok(())
}
Expand Down Expand Up @@ -151,7 +133,6 @@ impl PciDevices {
virtio_device.allocate_bars(
&mut resource_allocator.mmio32_memory,
&mut resource_allocator.mmio64_memory,
None,
)?;

let virtio_device = Arc::new(Mutex::new(virtio_device));
Expand Down
Loading