Skip to content

Commit cbe3edc

Browse files
committed
refactor(pci): assume we only have a single 64-bit BAR
In our case, only VirtIO PCI devices use a BAR region. Moreover, this region has a fixed size and it's always a 64-bit region. This means we don't handle 32-bit or IO BARs and we don't support a ROM bar. Encode these assumptions to the logic that adds BAR regions and detects BAR reprogramming attempts from the guest side so that we simplify the code. Signed-off-by: Babis Chalios <bchalios@amazon.es>
1 parent c4be916 commit cbe3edc

File tree

6 files changed

+185
-425
lines changed

6 files changed

+185
-425
lines changed

src/pci/src/bus.rs

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,6 @@ impl PciConfigIo {
213213
params.new_base,
214214
params.len,
215215
device.deref_mut(),
216-
params.region_type,
217216
) {
218217
error!(
219218
"Failed moving device BAR: {}: 0x{:x}->0x{:x}(0x{:x})",
@@ -348,7 +347,6 @@ impl PciConfigMmio {
348347
params.new_base,
349348
params.len,
350349
device.deref_mut(),
351-
params.region_type,
352350
) {
353351
error!(
354352
"Failed moving device BAR: {}: 0x{:x}->0x{:x}(0x{:x})",
@@ -445,8 +443,8 @@ mod tests {
445443
use super::{PciBus, PciConfigIo, PciConfigMmio, PciRoot};
446444
use crate::bus::{DEVICE_ID_INTEL_VIRT_PCIE_HOST, VENDOR_ID_INTEL};
447445
use crate::{
448-
DeviceRelocation, PciBarConfiguration, PciBarPrefetchable, PciBarRegionType, PciClassCode,
449-
PciConfiguration, PciDevice, PciHeaderType, PciMassStorageSubclass,
446+
DeviceRelocation, PciClassCode, PciConfiguration, PciDevice, PciHeaderType,
447+
PciMassStorageSubclass,
450448
};
451449

452450
#[derive(Debug, Default)]
@@ -467,7 +465,6 @@ mod tests {
467465
_new_base: u64,
468466
_len: u64,
469467
_pci_dev: &mut dyn crate::PciDevice,
470-
_region_type: crate::PciBarRegionType,
471468
) -> std::result::Result<(), std::io::Error> {
472469
self.reloc_cnt
473470
.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
@@ -493,15 +490,7 @@ mod tests {
493490
None,
494491
);
495492

496-
config
497-
.add_pci_bar(&PciBarConfiguration {
498-
addr: 0x1000,
499-
size: 0x1000,
500-
idx: 0,
501-
region_type: PciBarRegionType::Memory32BitRegion,
502-
prefetchable: PciBarPrefetchable::Prefetchable,
503-
})
504-
.unwrap();
493+
config.add_pci_bar(0, 0x1000, 0x1000).unwrap();
505494

506495
PciDevMock(config)
507496
}
@@ -920,23 +909,32 @@ mod tests {
920909
read_mmio_config(&mut mmio_config, 0, 1, 0, 0x4, 0, &mut buffer);
921910
let old_addr = u32::from_le_bytes(buffer) & 0xffff_fff0;
922911
assert_eq!(old_addr, 0x1000);
912+
913+
// Writing the lower 32bits first should not trigger any reprogramming
923914
write_mmio_config(
924915
&mut mmio_config,
925916
0,
926917
1,
927918
0,
928919
0x4,
929920
0,
930-
&u32::to_le_bytes(0x1312_1110),
921+
&u32::to_le_bytes(0x1312_0000),
931922
);
932923

933924
read_mmio_config(&mut mmio_config, 0, 1, 0, 0x4, 0, &mut buffer);
934925
let new_addr = u32::from_le_bytes(buffer) & 0xffff_fff0;
935-
assert_eq!(new_addr, 0x1312_1110);
936-
assert_eq!(mock.cnt(), 1);
926+
assert_eq!(new_addr, 0x1312_0000);
927+
assert_eq!(mock.cnt(), 0);
937928

938-
// BAR1 should not be used, so reading its address should return all 0s
929+
// Writing the upper 32bits first should now trigger the reprogramming logic
930+
write_mmio_config(&mut mmio_config, 0, 1, 0, 0x5, 0, &u32::to_le_bytes(0x1110));
939931
read_mmio_config(&mut mmio_config, 0, 1, 0, 0x5, 0, &mut buffer);
932+
let new_addr = u32::from_le_bytes(buffer);
933+
assert_eq!(new_addr, 0x1110);
934+
assert_eq!(mock.cnt(), 1);
935+
936+
// BAR2 should not be used, so reading its address should return all 0s
937+
read_mmio_config(&mut mmio_config, 0, 1, 0, 0x6, 0, &mut buffer);
940938
assert_eq!(buffer, [0x0, 0x0, 0x0, 0x0]);
941939

942940
// and reprogramming shouldn't have any effect
@@ -950,7 +948,7 @@ mod tests {
950948
&u32::to_le_bytes(0x1312_1110),
951949
);
952950

953-
read_mmio_config(&mut mmio_config, 0, 1, 0, 0x5, 0, &mut buffer);
951+
read_mmio_config(&mut mmio_config, 0, 1, 0, 0x6, 0, &mut buffer);
954952
assert_eq!(buffer, [0x0, 0x0, 0x0, 0x0]);
955953
}
956954
}

0 commit comments

Comments
 (0)