Skip to content

Commit 88bde66

Browse files
committed
pci: rollback BAR address on failed move_bar
When BAR reprogramming is detected, detect_bar_reprogramming() eagerly updates the BAR address in config space before the actual MMIO remapping occurs. If the subsequent move_bar() fails (e.g. the new address falls outside the allocator range), the config register retains the new address while the MMIO bus still uses the old one, leaving the device broken. Add restore_bar_addr() to undo the config space update when move_bar() fails, so the device remains functional at its original address. For 64-bit BARs, restore both the low and high BAR slots as well as the corresponding config registers, mirroring the two-slot update logic in detect_bar_reprogramming(). The VirtioPciDevice::restore_bar_addr() delegates directly to PciConfiguration since DeviceRelocation::move_bar() calls pci_dev.move_bar() last — if the relocation fails earlier, bar_regions was never updated and needs no rollback. Signed-off-by: CMGS <ilskdw@gmail.com>
1 parent 7461143 commit 88bde66

4 files changed

Lines changed: 68 additions & 5 deletions

File tree

pci/src/bus.rs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use std::ops::DerefMut;
1010
use std::sync::{Arc, Barrier, Mutex};
1111

1212
use byteorder::{ByteOrder, LittleEndian};
13-
use log::error;
13+
use log::warn;
1414
use thiserror::Error;
1515
use vm_device::{Bus, BusDevice, BusDeviceSync};
1616

@@ -280,10 +280,15 @@ impl PciConfigIo {
280280
device.deref_mut(),
281281
params.region_type,
282282
) {
283-
error!(
284-
"Failed moving device BAR: {}: 0x{:x}->0x{:x}(0x{:x})",
283+
warn!(
284+
"Failed moving device BAR: {}: 0x{:x}->0x{:x}(0x{:x}), keeping old BAR",
285285
e, params.old_base, params.new_base, params.len
286286
);
287+
// Rollback: the config register was already updated to
288+
// new_base by detect_bar_reprogramming(). Restore it by
289+
// writing back the old address so device state stays
290+
// consistent with the MMIO bus mapping.
291+
device.restore_bar_addr(params);
287292
}
288293
}
289294

@@ -405,10 +410,11 @@ impl PciConfigMmio {
405410
device.deref_mut(),
406411
params.region_type,
407412
) {
408-
error!(
409-
"Failed moving device BAR: {}: 0x{:x}->0x{:x}(0x{:x})",
413+
warn!(
414+
"Failed moving device BAR: {}: 0x{:x}->0x{:x}(0x{:x}), keeping old BAR",
410415
e, params.old_base, params.new_base, params.len
411416
);
417+
device.restore_bar_addr(params);
412418
}
413419
}
414420
}

pci/src/configuration.rs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1093,6 +1093,55 @@ impl PciConfiguration {
10931093
pub(crate) fn clear_pending_bar_reprogram(&mut self) {
10941094
self.pending_bar_reprogram = Vec::new();
10951095
}
1096+
1097+
/// Restore BAR address after a failed move. This undoes the premature
1098+
/// address update in detect_bar_reprogramming() so that config space
1099+
/// stays consistent with the actual MMIO mapping.
1100+
pub fn restore_bar_addr(&mut self, params: &BarReprogrammingParams) {
1101+
match params.region_type {
1102+
PciBarRegionType::Memory64BitRegion => {
1103+
// 64-bit BAR spans two slots: bars[i] (low, type Memory64BitRegion)
1104+
// and bars[i+1] (high, type None). Mirror detect_bar_reprogramming
1105+
// by matching the combined address and restoring both halves.
1106+
for i in 0..NUM_BAR_REGS - 1 {
1107+
if self.bars[i].r#type != Some(PciBarRegionType::Memory64BitRegion) {
1108+
continue;
1109+
}
1110+
let low_mask = self.writable_bits[BAR0_REG + i];
1111+
let high_mask = self.writable_bits[BAR0_REG + i + 1];
1112+
let current = (u64::from(self.bars[i + 1].addr & high_mask) << 32)
1113+
| u64::from(self.bars[i].addr & low_mask);
1114+
if current == params.new_base {
1115+
let old_low = params.old_base as u32;
1116+
let old_high = (params.old_base >> 32) as u32;
1117+
self.bars[i].addr = old_low;
1118+
self.bars[i + 1].addr = old_high;
1119+
self.registers[BAR0_REG + i] =
1120+
(self.registers[BAR0_REG + i] & !low_mask) | (old_low & low_mask);
1121+
self.registers[BAR0_REG + i + 1] = (self.registers[BAR0_REG + i + 1]
1122+
& !high_mask)
1123+
| (old_high & high_mask);
1124+
return;
1125+
}
1126+
}
1127+
}
1128+
_ => {
1129+
// 32-bit Memory or IO BAR
1130+
for i in 0..NUM_BAR_REGS {
1131+
let mask = self.writable_bits[BAR0_REG + i];
1132+
if self.bars[i].r#type == Some(params.region_type)
1133+
&& u64::from(self.bars[i].addr & mask) == params.new_base
1134+
{
1135+
let old = params.old_base as u32;
1136+
self.bars[i].addr = old;
1137+
self.registers[BAR0_REG + i] =
1138+
(self.registers[BAR0_REG + i] & !mask) | (old & mask);
1139+
return;
1140+
}
1141+
}
1142+
}
1143+
}
1144+
}
10961145
}
10971146

10981147
impl Pausable for PciConfiguration {}

pci/src/device.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,10 @@ pub trait PciDevice: Send {
9393
fn move_bar(&mut self, _old_base: u64, _new_base: u64) -> result::Result<(), io::Error> {
9494
Ok(())
9595
}
96+
/// Restore BAR address in config space after a failed move_bar.
97+
/// This rolls back the address update made by detect_bar_reprogramming()
98+
/// so that the config register stays consistent with the MMIO bus mapping.
99+
fn restore_bar_addr(&mut self, _params: &BarReprogrammingParams) {}
96100
/// Provides a mutable reference to the Any trait. This is useful to let
97101
/// the caller have access to the underlying type behind the trait.
98102
fn as_any_mut(&mut self) -> &mut dyn Any;

virtio-devices/src/transport/pci_device.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1133,6 +1133,10 @@ impl PciDevice for VirtioPciDevice {
11331133
Ok(())
11341134
}
11351135

1136+
fn restore_bar_addr(&mut self, params: &BarReprogrammingParams) {
1137+
self.configuration.restore_bar_addr(params);
1138+
}
1139+
11361140
fn read_bar(&mut self, _base: u64, offset: u64, data: &mut [u8]) {
11371141
match offset {
11381142
o if o < COMMON_CONFIG_BAR_OFFSET + COMMON_CONFIG_SIZE => self.common_config.read(

0 commit comments

Comments
 (0)