pci: Reprogram device BAR when its MSE bit is set

The Memory Space Enable (MSE) bit from the COMMAND register in the
PCI configuration space controls whether a PCI device responds to memory
space accesses, e.g. read and write cycles to the device MMIO regions
defined by its BARs. The MSE bit is used by the device drivers to ensure
the correctness of BAR reprogramming. A common workflow is, the driver
first clears the MSE bit, then writes new values to the BAR registers,
and finally set the MSE bit to finish the BAR reprogramming.

This patch changes how we handle BAR reprogramming for all PCI
devices (e.g. virtio-pci, vfio, vfio-user, etc.), so that we follow the
same convention, e.g. moving PCI BARs when its MSE bit is set.

Note that some device drivers (such as edk2) only clear and set MSE once
while reprogramming multiple BARs of a single device. To support such
behavior, this patch adds support for multiple pending BAR reprogramming.

See: https://github.com/cloud-hypervisor/cloud-hypervisor/issues/7027#issuecomment-2853642959

Signed-off-by: Bo Chen <bchen@crusoe.ai>
This commit is contained in:
Bo Chen
2025-05-12 22:15:56 +00:00
parent 59f98a2edc
commit aaf86ef209
8 changed files with 43 additions and 17 deletions

View File

@@ -698,7 +698,7 @@ impl PciDevice for PvmemcontrolPciDevice {
reg_idx: usize,
offset: u64,
data: &[u8],
) -> (Option<BarReprogrammingParams>, Option<Arc<Barrier>>) {
) -> (Vec<BarReprogrammingParams>, Option<Arc<Barrier>>) {
(
self.configuration
.write_config_register(reg_idx, offset, data),

View File

@@ -90,7 +90,7 @@ impl PvPanicDevice {
let command: [u8; 2] = [0x03, 0x01];
let bar_reprogram = configuration.write_config_register(1, 0, &command);
assert!(
bar_reprogram.is_none(),
bar_reprogram.is_empty(),
"No bar reprogrammig is expected from writing to the COMMAND register"
);
@@ -160,7 +160,7 @@ impl PciDevice for PvPanicDevice {
reg_idx: usize,
offset: u64,
data: &[u8],
) -> (Option<BarReprogrammingParams>, Option<Arc<Barrier>>) {
) -> (Vec<BarReprogrammingParams>, Option<Arc<Barrier>>) {
(
self.configuration
.write_config_register(reg_idx, offset, data),

View File

@@ -81,7 +81,7 @@ impl PciDevice for PciRoot {
reg_idx: usize,
offset: u64,
data: &[u8],
) -> (Option<BarReprogrammingParams>, Option<Arc<Barrier>>) {
) -> (Vec<BarReprogrammingParams>, Option<Arc<Barrier>>) {
(
self.config.write_config_register(reg_idx, offset, data),
None,
@@ -262,7 +262,7 @@ impl PciConfigIo {
let (bar_reprogram, ret) = device.write_config_register(register, offset, data);
// Move the device's BAR if needed
if let Some(params) = bar_reprogram {
for params in &bar_reprogram {
if let Err(e) = pci_bus.device_reloc.move_bar(
params.old_base,
params.new_base,
@@ -387,7 +387,7 @@ impl PciConfigMmio {
let (bar_reprogram, _) = device.write_config_register(register, offset, data);
// Move the device's BAR if needed
if let Some(params) = bar_reprogram {
for params in &bar_reprogram {
if let Err(e) = pci_bus.device_reloc.move_bar(
params.old_base,
params.new_base,

View File

@@ -18,6 +18,8 @@ use crate::{MsixConfig, PciInterruptPin};
// The number of 32bit registers in the config space, 4096 bytes.
const NUM_CONFIGURATION_REGISTERS: usize = 1024;
const COMMAND_REG: usize = 1;
const COMMAND_REG_MEMORY_SPACE_MASK: u32 = 0x0000_0002;
const STATUS_REG: usize = 1;
const STATUS_REG_CAPABILITIES_USED_MASK: u32 = 0x0010_0000;
const BAR0_REG: usize = 4;
@@ -436,6 +438,7 @@ pub struct PciConfiguration {
last_capability: Option<(usize, usize)>,
msix_cap_reg_idx: Option<usize>,
msix_config: Option<Arc<Mutex<MsixConfig>>>,
pending_bar_reprogram: Vec<BarReprogrammingParams>,
}
/// See pci_regs.h in kernel
@@ -630,6 +633,7 @@ impl PciConfiguration {
last_capability,
msix_cap_reg_idx,
msix_config,
pending_bar_reprogram: Vec::new(),
}
}
@@ -912,9 +916,9 @@ impl PciConfiguration {
reg_idx: usize,
offset: u64,
data: &[u8],
) -> Option<BarReprogrammingParams> {
) -> Vec<BarReprogrammingParams> {
if offset as usize + data.len() > 4 {
return None;
return Vec::new();
}
// Handle potential write to MSI-X message control register
@@ -944,7 +948,29 @@ impl PciConfiguration {
_ => (),
}
self.detect_bar_reprogramming(reg_idx, data)
if let Some(param) = self.detect_bar_reprogramming(reg_idx, data) {
self.pending_bar_reprogram.push(param);
}
if !self.pending_bar_reprogram.is_empty() {
// Return bar reprogramming only if the MSE bit is enabled;
if self.read_config_register(COMMAND_REG) & COMMAND_REG_MEMORY_SPACE_MASK
== COMMAND_REG_MEMORY_SPACE_MASK
{
info!(
"BAR reprogramming parameter is returned: {:x?}",
self.pending_bar_reprogram
);
return self.pending_bar_reprogram.drain(..).collect();
} else {
info!(
"MSE bit is disabled. No BAR reprogramming parameter is returned: {:x?}",
self.pending_bar_reprogram
);
}
}
Vec::new()
}
pub fn read_config_register(&self, reg_idx: usize) -> u32 {

View File

@@ -48,7 +48,7 @@ impl Display for Error {
}
}
#[derive(Clone, Copy)]
#[derive(Clone, Copy, Debug)]
pub struct BarReprogrammingParams {
pub old_base: u64,
pub new_base: u64,
@@ -87,7 +87,7 @@ pub trait PciDevice: Send {
reg_idx: usize,
offset: u64,
data: &[u8],
) -> (Option<BarReprogrammingParams>, Option<Arc<Barrier>>);
) -> (Vec<BarReprogrammingParams>, Option<Arc<Barrier>>);
/// Gets a register from the configuration space.
/// * `reg_idx` - The index of the config register to read.
fn read_config_register(&mut self, reg_idx: usize) -> u32;

View File

@@ -1242,7 +1242,7 @@ impl VfioCommon {
reg_idx: usize,
offset: u64,
data: &[u8],
) -> (Option<BarReprogrammingParams>, Option<Arc<Barrier>>) {
) -> (Vec<BarReprogrammingParams>, Option<Arc<Barrier>>) {
// When the guest wants to write to a BAR, we trap it into
// our local configuration space. We're not reprogramming
// VFIO device.
@@ -1292,7 +1292,7 @@ impl VfioCommon {
// to the device region to update the MSI Enable bit.
self.vfio_wrapper.write_config((reg + offset) as u32, data);
(None, None)
(Vec::new(), None)
}
pub(crate) fn read_config_register(&mut self, reg_idx: usize) -> u32 {
@@ -1852,7 +1852,7 @@ impl PciDevice for VfioPciDevice {
reg_idx: usize,
offset: u64,
data: &[u8],
) -> (Option<BarReprogrammingParams>, Option<Arc<Barrier>>) {
) -> (Vec<BarReprogrammingParams>, Option<Arc<Barrier>>) {
self.common.write_config_register(reg_idx, offset, data)
}

View File

@@ -431,7 +431,7 @@ impl PciDevice for VfioUserPciDevice {
reg_idx: usize,
offset: u64,
data: &[u8],
) -> (Option<BarReprogrammingParams>, Option<Arc<Barrier>>) {
) -> (Vec<BarReprogrammingParams>, Option<Arc<Barrier>>) {
self.common.write_config_register(reg_idx, offset, data)
}

View File

@@ -912,7 +912,7 @@ impl PciDevice for VirtioPciDevice {
reg_idx: usize,
offset: u64,
data: &[u8],
) -> (Option<BarReprogrammingParams>, Option<Arc<Barrier>>) {
) -> (Vec<BarReprogrammingParams>, 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.
@@ -922,7 +922,7 @@ impl PciDevice for VirtioPciDevice {
<= self.cap_pci_cfg_info.offset + self.cap_pci_cfg_info.cap.bytes().len()
{
let offset = base + offset as usize - self.cap_pci_cfg_info.offset;
(None, self.write_cap_pci_cfg(offset, data))
(Vec::new(), self.write_cap_pci_cfg(offset, data))
} else {
(
self.configuration