Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 094f394

Browse files
committed
fix(pci): handling of PCI ISR capability
We always enable MSI-X interrupts for VirtIO devices when using the PCI transport. Currently, we don't offer a fallback if the driver doesn't want to use MSI-X, but VirtIO drivers we are working with always support that. So this should not be a problem for our use-cases in the kernels we support at the moment. On the other hand, despite not supporting legacy interrupts, we still did some maintenance for the ISR status byte which is not used by MSI-X interrupts at all. Drop all this handling, ensure that reads always return 0 and writes have no effect in the state of the device. Also, add warning messages for the cases where the guest tries to access the ISR parts of the BAR. (cherry picked from commit 1dee309) Signed-off-by: Babis Chalios <bchalios@amazon.es>
1 parent 69854b6 commit 094f394

File tree

1 file changed

+29
-17
lines changed
  • src/vmm/src/devices/virtio/transport/pci

1 file changed

+29
-17
lines changed

‎src/vmm/src/devices/virtio/transport/pci/device.rs

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use std::sync::{Arc, Barrier, Mutex};
1616

1717
use anyhow::anyhow;
1818
use kvm_ioctls::{IoEventAddress, NoDatamatch};
19+
use log::warn;
1920
use pci::{
2021
BarReprogrammingParams, MsixCap, MsixConfig, MsixConfigState, PciBarConfiguration,
2122
PciBarRegionType, PciBdf, PciCapability, PciCapabilityId, PciClassCode, PciConfiguration,
@@ -239,7 +240,6 @@ const VIRTIO_PCI_DEVICE_ID_BASE: u16 = 0x1040; // Add to device type to get devi
239240
pub struct VirtioPciDeviceState {
240241
pub pci_device_bdf: PciBdf,
241242
pub device_activated: bool,
242-
pub interrupt_status: usize,
243243
pub cap_pci_cfg_offset: usize,
244244
pub cap_pci_cfg: Vec<u8>,
245245
pub pci_configuration_state: PciConfigurationState,
@@ -281,7 +281,6 @@ pub struct VirtioPciDevice {
281281
device_activated: Arc<AtomicBool>,
282282

283283
// PCI interrupts.
284-
interrupt_status: Arc<AtomicUsize>,
285284
virtio_interrupt: Option<Arc<dyn VirtioInterrupt>>,
286285
interrupt_source_group: Arc<MsiVectorGroup>,
287286

@@ -402,7 +401,6 @@ impl VirtioPciDevice {
402401
msix_num: msi_vectors.num_vectors(),
403402
device,
404403
device_activated: Arc::new(AtomicBool::new(false)),
405-
interrupt_status: Arc::new(AtomicUsize::new(0)),
406404
virtio_interrupt: Some(interrupt),
407405
memory,
408406
interrupt_source_group: msi_vectors,
@@ -453,7 +451,6 @@ impl VirtioPciDevice {
453451
msix_num: msi_vectors.num_vectors(),
454452
device,
455453
device_activated: Arc::new(AtomicBool::new(state.device_activated)),
456-
interrupt_status: Arc::new(AtomicUsize::new(state.interrupt_status)),
457454
virtio_interrupt: Some(interrupt),
458455
memory: memory.clone(),
459456
interrupt_source_group: msi_vectors,
@@ -640,7 +637,6 @@ impl VirtioPciDevice {
640637
VirtioPciDeviceState {
641638
pci_device_bdf: self.pci_device_bdf,
642639
device_activated: self.device_activated.load(Ordering::Acquire),
643-
interrupt_status: self.interrupt_status.load(Ordering::Acquire),
644640
cap_pci_cfg_offset: self.cap_pci_cfg_info.offset,
645641
cap_pci_cfg: self.cap_pci_cfg_info.cap.bytes().to_vec(),
646642
pci_configuration_state: self.configuration.state(),
@@ -864,14 +860,9 @@ impl PciDevice for VirtioPciDevice {
864860
.read(o - COMMON_CONFIG_BAR_OFFSET, data, self.device.clone())
865861
}
866862
o if (ISR_CONFIG_BAR_OFFSET..ISR_CONFIG_BAR_OFFSET + ISR_CONFIG_SIZE).contains(&o) => {
867-
if let Some(v) = data.get_mut(0) {
868-
// Reading this register resets it to 0.
869-
*v = self
870-
.interrupt_status
871-
.swap(0, Ordering::AcqRel)
872-
.try_into()
873-
.unwrap();
874-
}
863+
// We don't actually support legacy INT#x interrupts for VirtIO PCI devices
864+
warn!("pci: read access to unsupported ISR status field");
865+
data.fill(0);
875866
}
876867
o if (DEVICE_CONFIG_BAR_OFFSET..DEVICE_CONFIG_BAR_OFFSET + DEVICE_CONFIG_SIZE)
877868
.contains(&o) =>
@@ -911,10 +902,8 @@ impl PciDevice for VirtioPciDevice {
911902
.write(o - COMMON_CONFIG_BAR_OFFSET, data, self.device.clone())
912903
}
913904
o if (ISR_CONFIG_BAR_OFFSET..ISR_CONFIG_BAR_OFFSET + ISR_CONFIG_SIZE).contains(&o) => {
914-
if let Some(v) = data.first() {
915-
self.interrupt_status
916-
.fetch_and(!(*v as usize), Ordering::AcqRel);
917-
}
905+
// We don't actually support legacy INT#x interrupts for VirtIO PCI devices
906+
warn!("pci: access to unsupported ISR status field");
918907
}
919908
o if (DEVICE_CONFIG_BAR_OFFSET..DEVICE_CONFIG_BAR_OFFSET + DEVICE_CONFIG_SIZE)
920909
.contains(&o) =>
@@ -1460,4 +1449,27 @@ mod tests {
14601449
0x42
14611450
);
14621451
}
1452+
1453+
fn isr_status_read(device: &mut VirtioPciDevice) -> u32 {
1454+
let mut data = 0u32;
1455+
device.read_bar(0, ISR_CONFIG_BAR_OFFSET, data.as_mut_slice());
1456+
data
1457+
}
1458+
1459+
fn isr_status_write(device: &mut VirtioPciDevice, data: u32) {
1460+
device.write_bar(0, ISR_CONFIG_BAR_OFFSET, data.as_slice());
1461+
}
1462+
1463+
#[test]
1464+
fn test_isr_capability() {
1465+
let mut vmm = create_vmm_with_virtio_pci_device();
1466+
let device = get_virtio_device(&vmm);
1467+
let mut locked_virtio_pci_device = device.lock().unwrap();
1468+
1469+
// We don't support legacy interrupts so reads to ISR BAR should always return 0s and
1470+
// writes to it should not have any effect
1471+
assert_eq!(isr_status_read(&mut locked_virtio_pci_device), 0);
1472+
isr_status_write(&mut locked_virtio_pci_device, 0x1312);
1473+
assert_eq!(isr_status_read(&mut locked_virtio_pci_device), 0);
1474+
}
14631475
}

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /