pci: Clarify register index and BAR index

The code was quite unclear regarding the type of index that was being
used regarding a BAR. This is improved by differenciating register
indexes and BAR indexes more clearly.

Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
This commit is contained in:
Sebastien Boeuf 2022-04-14 17:28:38 +02:00 committed by Bo Chen
parent 1795afadb8
commit da95c0d784
3 changed files with 60 additions and 59 deletions

View File

@ -18,6 +18,7 @@ const STATUS_REG: usize = 1;
const STATUS_REG_CAPABILITIES_USED_MASK: u32 = 0x0010_0000; const STATUS_REG_CAPABILITIES_USED_MASK: u32 = 0x0010_0000;
const BAR0_REG: usize = 4; const BAR0_REG: usize = 4;
const ROM_BAR_REG: usize = 12; const ROM_BAR_REG: usize = 12;
const ROM_BAR_IDX: usize = 6;
const BAR_IO_ADDR_MASK: u32 = 0xffff_fffc; const BAR_IO_ADDR_MASK: u32 = 0xffff_fffc;
const BAR_MEM_ADDR_MASK: u32 = 0xffff_fff0; const BAR_MEM_ADDR_MASK: u32 = 0xffff_fff0;
const ROM_BAR_ADDR_MASK: u32 = 0xffff_f800; const ROM_BAR_ADDR_MASK: u32 = 0xffff_f800;
@ -336,7 +337,7 @@ pub enum PciBarPrefetchable {
pub struct PciBarConfiguration { pub struct PciBarConfiguration {
addr: u64, addr: u64,
size: u64, size: u64,
reg_idx: usize, idx: usize,
region_type: PciBarRegionType, region_type: PciBarRegionType,
prefetchable: PciBarPrefetchable, prefetchable: PciBarPrefetchable,
} }
@ -556,22 +557,23 @@ impl PciConfiguration {
/// Adds a region specified by `config`. Configures the specified BAR(s) to /// Adds a region specified by `config`. Configures the specified BAR(s) to
/// report this region and size to the guest kernel. Enforces a few constraints /// report this region and size to the guest kernel. Enforces a few constraints
/// (i.e, region size must be power of two, register not already used). Returns 'None' on /// (i.e, region size must be power of two, register not already used).
/// failure all, `Some(BarIndex)` on success. pub fn add_pci_bar(&mut self, config: &PciBarConfiguration) -> Result<()> {
pub fn add_pci_bar(&mut self, config: &PciBarConfiguration) -> Result<usize> { let bar_idx = config.idx;
if self.bars[config.reg_idx].used { let reg_idx = BAR0_REG + bar_idx;
return Err(Error::BarInUse(config.reg_idx));
if self.bars[bar_idx].used {
return Err(Error::BarInUse(bar_idx));
} }
if config.size.count_ones() != 1 { if config.size.count_ones() != 1 {
return Err(Error::BarSizeInvalid(config.size)); return Err(Error::BarSizeInvalid(config.size));
} }
if config.reg_idx >= NUM_BAR_REGS { if bar_idx >= NUM_BAR_REGS {
return Err(Error::BarInvalid(config.reg_idx)); return Err(Error::BarInvalid(bar_idx));
} }
let bar_idx = BAR0_REG + config.reg_idx;
let end_addr = config let end_addr = config
.addr .addr
.checked_add(config.size - 1) .checked_add(config.size - 1)
@ -584,20 +586,20 @@ impl PciConfiguration {
// Encode the BAR size as expected by the software running in // Encode the BAR size as expected by the software running in
// the guest. // the guest.
self.bars[config.reg_idx].size = self.bars[bar_idx].size =
encode_32_bits_bar_size(config.size as u32).ok_or(Error::Encode32BarSize)?; encode_32_bits_bar_size(config.size as u32).ok_or(Error::Encode32BarSize)?;
} }
PciBarRegionType::Memory64BitRegion => { PciBarRegionType::Memory64BitRegion => {
if config.reg_idx + 1 >= NUM_BAR_REGS { if bar_idx + 1 >= NUM_BAR_REGS {
return Err(Error::BarInvalid64(config.reg_idx)); return Err(Error::BarInvalid64(bar_idx));
} }
if end_addr > u64::max_value() { if end_addr > u64::max_value() {
return Err(Error::BarAddressInvalid(config.addr, config.size)); return Err(Error::BarAddressInvalid(config.addr, config.size));
} }
if self.bars[config.reg_idx + 1].used { if self.bars[bar_idx + 1].used {
return Err(Error::BarInUse64(config.reg_idx)); return Err(Error::BarInUse64(bar_idx));
} }
// Encode the BAR size as expected by the software running in // Encode the BAR size as expected by the software running in
@ -605,12 +607,12 @@ impl PciConfiguration {
let (bar_size_hi, bar_size_lo) = let (bar_size_hi, bar_size_lo) =
encode_64_bits_bar_size(config.size).ok_or(Error::Encode64BarSize)?; encode_64_bits_bar_size(config.size).ok_or(Error::Encode64BarSize)?;
self.registers[bar_idx + 1] = (config.addr >> 32) as u32; self.registers[reg_idx + 1] = (config.addr >> 32) as u32;
self.writable_bits[bar_idx + 1] = 0xffff_ffff; self.writable_bits[reg_idx + 1] = 0xffff_ffff;
self.bars[config.reg_idx + 1].addr = self.registers[bar_idx + 1]; self.bars[bar_idx + 1].addr = self.registers[reg_idx + 1];
self.bars[config.reg_idx].size = bar_size_lo; self.bars[bar_idx].size = bar_size_lo;
self.bars[config.reg_idx + 1].size = bar_size_hi; self.bars[bar_idx + 1].size = bar_size_hi;
self.bars[config.reg_idx + 1].used = true; self.bars[bar_idx + 1].used = true;
} }
} }
@ -622,26 +624,30 @@ impl PciConfiguration {
PciBarRegionType::IoRegion => (BAR_IO_ADDR_MASK, config.region_type as u32), PciBarRegionType::IoRegion => (BAR_IO_ADDR_MASK, config.region_type as u32),
}; };
self.registers[bar_idx] = ((config.addr as u32) & mask) | lower_bits; self.registers[reg_idx] = ((config.addr as u32) & mask) | lower_bits;
self.writable_bits[bar_idx] = mask; self.writable_bits[reg_idx] = mask;
self.bars[config.reg_idx].addr = self.registers[bar_idx]; self.bars[bar_idx].addr = self.registers[reg_idx];
self.bars[config.reg_idx].used = true; self.bars[bar_idx].used = true;
self.bars[config.reg_idx].r#type = Some(config.region_type); self.bars[bar_idx].r#type = Some(config.region_type);
Ok(config.reg_idx)
Ok(())
} }
/// Adds rom expansion BAR. /// Adds rom expansion BAR.
pub fn add_pci_rom_bar(&mut self, config: &PciBarConfiguration, active: u32) -> Result<usize> { 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 { if self.rom_bar_used {
return Err(Error::RomBarInUse(config.reg_idx)); return Err(Error::RomBarInUse(bar_idx));
} }
if config.size.count_ones() != 1 { if config.size.count_ones() != 1 {
return Err(Error::RomBarSizeInvalid(config.size)); return Err(Error::RomBarSizeInvalid(config.size));
} }
if config.reg_idx != ROM_BAR_REG { if bar_idx != ROM_BAR_IDX {
return Err(Error::RomBarInvalid(config.reg_idx)); return Err(Error::RomBarInvalid(bar_idx));
} }
let end_addr = config let end_addr = config
@ -653,13 +659,14 @@ impl PciConfiguration {
return Err(Error::RomBarAddressInvalid(config.addr, config.size)); return Err(Error::RomBarAddressInvalid(config.addr, config.size));
} }
self.registers[config.reg_idx] = (config.addr as u32) | active; self.registers[reg_idx] = (config.addr as u32) | active;
self.writable_bits[config.reg_idx] = ROM_BAR_ADDR_MASK; self.writable_bits[reg_idx] = ROM_BAR_ADDR_MASK;
self.rom_bar_addr = self.registers[config.reg_idx]; self.rom_bar_addr = self.registers[reg_idx];
self.rom_bar_size = self.rom_bar_size =
encode_32_bits_bar_size(config.size as u32).ok_or(Error::Encode32BarSize)?; encode_32_bits_bar_size(config.size as u32).ok_or(Error::Encode32BarSize)?;
self.rom_bar_used = true; self.rom_bar_used = true;
Ok(config.reg_idx)
Ok(())
} }
/// Returns the address of the given BAR region. /// Returns the address of the given BAR region.
@ -911,7 +918,7 @@ impl Snapshottable for PciConfiguration {
impl Default for PciBarConfiguration { impl Default for PciBarConfiguration {
fn default() -> Self { fn default() -> Self {
PciBarConfiguration { PciBarConfiguration {
reg_idx: 0, idx: 0,
addr: 0, addr: 0,
size: 0, size: 0,
region_type: PciBarRegionType::Memory64BitRegion, region_type: PciBarRegionType::Memory64BitRegion,
@ -922,13 +929,13 @@ impl Default for PciBarConfiguration {
impl PciBarConfiguration { impl PciBarConfiguration {
pub fn new( pub fn new(
reg_idx: usize, idx: usize,
size: u64, size: u64,
region_type: PciBarRegionType, region_type: PciBarRegionType,
prefetchable: PciBarPrefetchable, prefetchable: PciBarPrefetchable,
) -> Self { ) -> Self {
PciBarConfiguration { PciBarConfiguration {
reg_idx, idx,
addr: 0, addr: 0,
size, size,
region_type, region_type,
@ -937,8 +944,8 @@ impl PciBarConfiguration {
} }
#[must_use] #[must_use]
pub fn set_register_index(mut self, reg_idx: usize) -> Self { pub fn set_index(mut self, idx: usize) -> Self {
self.reg_idx = reg_idx; self.idx = idx;
self self
} }

View File

@ -498,15 +498,9 @@ impl VfioCommon {
.ok_or(PciDeviceError::IoAllocationFailed(region_size))?; .ok_or(PciDeviceError::IoAllocationFailed(region_size))?;
} }
let reg_idx = if bar_id == VFIO_PCI_ROM_REGION_INDEX {
PCI_ROM_EXP_BAR_INDEX
} else {
bar_id as usize
};
// We can now build our BAR configuration block. // We can now build our BAR configuration block.
let config = PciBarConfiguration::default() let config = PciBarConfiguration::default()
.set_register_index(reg_idx) .set_index(bar_id as usize)
.set_address(bar_addr.raw_value()) .set_address(bar_addr.raw_value())
.set_size(region_size) .set_size(region_size)
.set_region_type(region_type); .set_region_type(region_type);

View File

@ -260,6 +260,8 @@ const MSIX_PBA_BAR_OFFSET: u64 = 0x48000;
const MSIX_PBA_SIZE: u64 = 0x800; const MSIX_PBA_SIZE: u64 = 0x800;
// The BAR size must be a power of 2. // The BAR size must be a power of 2.
const CAPABILITY_BAR_SIZE: u64 = 0x80000; const CAPABILITY_BAR_SIZE: u64 = 0x80000;
const VIRTIO_COMMON_BAR_INDEX: usize = 0;
const VIRTIO_SHM_BAR_INDEX: usize = 2;
const NOTIFY_OFF_MULTIPLIER: u32 = 4; // A dword per notification address. const NOTIFY_OFF_MULTIPLIER: u32 = 4; // A dword per notification address.
@ -849,7 +851,7 @@ impl PciDevice for VirtioPciDevice {
if resources.is_empty() { if resources.is_empty() {
return Err(PciDeviceError::MissingResource); return Err(PciDeviceError::MissingResource);
} }
match resources[0] { match resources[VIRTIO_COMMON_BAR_INDEX] {
Resource::MmioAddressRange { base, .. } => Some(GuestAddress(base)), Resource::MmioAddressRange { base, .. } => Some(GuestAddress(base)),
_ => return Err(PciDeviceError::InvalidResourceType), _ => return Err(PciDeviceError::InvalidResourceType),
} }
@ -888,28 +890,26 @@ impl PciDevice for VirtioPciDevice {
.push((virtio_pci_bar_addr, CAPABILITY_BAR_SIZE, region_type)); .push((virtio_pci_bar_addr, CAPABILITY_BAR_SIZE, region_type));
let config = PciBarConfiguration::default() let config = PciBarConfiguration::default()
.set_register_index(0) .set_index(VIRTIO_COMMON_BAR_INDEX)
.set_address(virtio_pci_bar_addr.raw_value()) .set_address(virtio_pci_bar_addr.raw_value())
.set_size(CAPABILITY_BAR_SIZE) .set_size(CAPABILITY_BAR_SIZE)
.set_region_type(region_type); .set_region_type(region_type);
let virtio_pci_bar = self.configuration.add_pci_bar(&config).map_err(|e| {
self.configuration.add_pci_bar(&config).map_err(|e| { PciDeviceError::IoRegistrationFailed(virtio_pci_bar_addr.raw_value(), e)
PciDeviceError::IoRegistrationFailed(virtio_pci_bar_addr.raw_value(), e) })?;
})? as u8;
// Once the BARs are allocated, the capabilities can be added to the PCI configuration. // Once the BARs are allocated, the capabilities can be added to the PCI configuration.
self.add_pci_capabilities(virtio_pci_bar)?; self.add_pci_capabilities(VIRTIO_COMMON_BAR_INDEX as u8)?;
// Allocate a dedicated BAR if there are some shared memory regions. // Allocate a dedicated BAR if there are some shared memory regions.
if let Some(shm_list) = device.get_shm_regions() { if let Some(shm_list) = device.get_shm_regions() {
let config = PciBarConfiguration::default() let config = PciBarConfiguration::default()
.set_register_index(2) .set_index(VIRTIO_SHM_BAR_INDEX)
.set_address(shm_list.addr.raw_value()) .set_address(shm_list.addr.raw_value())
.set_size(shm_list.len); .set_size(shm_list.len);
let virtio_pci_shm_bar = self.configuration
self.configuration.add_pci_bar(&config).map_err(|e| { .add_pci_bar(&config)
PciDeviceError::IoRegistrationFailed(shm_list.addr.raw_value(), e) .map_err(|e| PciDeviceError::IoRegistrationFailed(shm_list.addr.raw_value(), e))?;
})? as u8;
let region_type = PciBarRegionType::Memory64BitRegion; let region_type = PciBarRegionType::Memory64BitRegion;
ranges.push((shm_list.addr, shm_list.len, region_type)); ranges.push((shm_list.addr, shm_list.len, region_type));
@ -919,7 +919,7 @@ impl PciDevice for VirtioPciDevice {
for (idx, shm) in shm_list.region_list.iter().enumerate() { for (idx, shm) in shm_list.region_list.iter().enumerate() {
let shm_cap = VirtioPciCap64::new( let shm_cap = VirtioPciCap64::new(
PciCapabilityType::SharedMemoryConfig, PciCapabilityType::SharedMemoryConfig,
virtio_pci_shm_bar, VIRTIO_SHM_BAR_INDEX as u8,
idx as u8, idx as u8,
shm.offset, shm.offset,
shm.len, shm.len,