Skip to content

Commit 6b9e315

Browse files
committed
pci: rollback BAR address on failed move_bar
When a guest (e.g. Windows) programs a BAR to an address outside the allocatable MMIO range, move_bar fails but the config register has already been updated by detect_bar_reprogramming. This inconsistency between config space and MMIO mapping causes virtio device activation to fail (too few queues enabled), crashing the VMM. Restore the BAR address in both config registers and internal bar_regions when move_bar fails, so the device remains functional at its original address. Signed-off-by: CMGS <ilskdw@gmail.com>
1 parent 7461143 commit 6b9e315

4 files changed

Lines changed: 40 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: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1093,6 +1093,20 @@ 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+
for bar in self.bars.iter_mut() {
1102+
if u64::from(bar.addr) == params.new_base
1103+
|| (u64::from(bar.addr) << 32) == params.new_base
1104+
{
1105+
bar.addr = params.old_base as u32;
1106+
break;
1107+
}
1108+
}
1109+
}
10961110
}
10971111

10981112
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: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1133,6 +1133,17 @@ impl PciDevice for VirtioPciDevice {
11331133
Ok(())
11341134
}
11351135

1136+
fn restore_bar_addr(&mut self, params: &BarReprogrammingParams) {
1137+
// Undo move_bar's bar_regions update
1138+
for bar in self.bar_regions.iter_mut() {
1139+
if bar.addr() == params.new_base {
1140+
*bar = bar.set_address(params.old_base);
1141+
}
1142+
}
1143+
// Undo config register update
1144+
self.configuration.restore_bar_addr(params);
1145+
}
1146+
11361147
fn read_bar(&mut self, _base: u64, offset: u64, data: &mut [u8]) {
11371148
match offset {
11381149
o if o < COMMON_CONFIG_BAR_OFFSET + COMMON_CONFIG_SIZE => self.common_config.read(

0 commit comments

Comments
 (0)