irq: Fix pin based interrupt for virtio-pci

When the KVM capability KVM_CAP_SIGNAL_MSI is not present, the VMM
falls back from MSI-X onto pin based interrupts. Unfortunately, this
was not working as expected because the VirtioPciDevice object was
always creating an MSI-X capability structure in the PCI configuration
space. This was causing the guest drivers to expect MSI-X interrupts
instead of the pin based generated ones.

This patch takes care of avoiding the creation of a dedicated MSI-X
capability structure when MSI is not supported by KVM.

Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
This commit is contained in:
Sebastien Boeuf 2019-06-07 09:19:46 -07:00 committed by Rob Bradford
parent 4be3dfeb37
commit 24dbe7003a
2 changed files with 70 additions and 51 deletions

View File

@ -171,7 +171,7 @@ pub struct VirtioPciDevice {
common_config: VirtioPciCommonConfig, common_config: VirtioPciCommonConfig,
// MSI-X config // MSI-X config
msix_config: Arc<Mutex<MsixConfig>>, msix_config: Option<Arc<Mutex<MsixConfig>>>,
// Number of MSI-X vectors // Number of MSI-X vectors
msix_num: u16, msix_num: u16,
@ -210,7 +210,13 @@ impl VirtioPciDevice {
let pci_device_id = VIRTIO_PCI_DEVICE_ID_BASE + device.device_type() as u16; let pci_device_id = VIRTIO_PCI_DEVICE_ID_BASE + device.device_type() as u16;
let msix_config = Arc::new(Mutex::new(MsixConfig::new(msix_num))); let (msix_config, msix_config_clone) = if msix_num > 0 {
let msix_config = Arc::new(Mutex::new(MsixConfig::new(msix_num)));
let msix_config_clone = msix_config.clone();
(Some(msix_config), Some(msix_config_clone))
} else {
(None, None)
};
let configuration = PciConfiguration::new( let configuration = PciConfiguration::new(
VIRTIO_PCI_VENDOR_ID, VIRTIO_PCI_VENDOR_ID,
@ -221,7 +227,7 @@ impl VirtioPciDevice {
PciHeaderType::Device, PciHeaderType::Device,
VIRTIO_PCI_VENDOR_ID, VIRTIO_PCI_VENDOR_ID,
pci_device_id, pci_device_id,
Some(msix_config.clone()), msix_config_clone,
); );
Ok(VirtioPciDevice { Ok(VirtioPciDevice {
@ -327,15 +333,17 @@ impl VirtioPciDevice {
.add_capability(&configuration_cap) .add_capability(&configuration_cap)
.map_err(PciDeviceError::CapabilitiesSetup)?; .map_err(PciDeviceError::CapabilitiesSetup)?;
let msix_cap = MsixCap::new( if self.msix_config.is_some() {
settings_bar, let msix_cap = MsixCap::new(
self.msix_num, settings_bar,
MSIX_TABLE_BAR_OFFSET as u32, self.msix_num,
MSIX_PBA_BAR_OFFSET as u32, MSIX_TABLE_BAR_OFFSET as u32,
); MSIX_PBA_BAR_OFFSET as u32,
self.configuration );
.add_capability(&msix_cap) self.configuration
.map_err(PciDeviceError::CapabilitiesSetup)?; .add_capability(&msix_cap)
.map_err(PciDeviceError::CapabilitiesSetup)?;
}
self.settings_bar = settings_bar; self.settings_bar = settings_bar;
Ok(()) Ok(())
@ -360,31 +368,33 @@ impl PciDevice for VirtioPciDevice {
} }
fn assign_msix(&mut self, msi_cb: Arc<InterruptDelivery>) { fn assign_msix(&mut self, msi_cb: Arc<InterruptDelivery>) {
self.msix_config if let Some(msix_config) = &self.msix_config {
.lock() msix_config
.unwrap() .lock()
.register_interrupt_cb(msi_cb.clone()); .unwrap()
.register_interrupt_cb(msi_cb.clone());
let msix_config = self.msix_config.clone(); let msix_config_clone = msix_config.clone();
let cb = Arc::new(Box::new(move |queue: &Queue| { let cb = Arc::new(Box::new(move |queue: &Queue| {
let config = &mut msix_config.lock().unwrap(); let config = &mut msix_config_clone.lock().unwrap();
let entry = &config.table_entries[queue.vector as usize]; let entry = &config.table_entries[queue.vector as usize];
// In case the vector control register associated with the entry // In case the vector control register associated with the entry
// has its first bit set, this means the vector is masked and the // has its first bit set, this means the vector is masked and the
// device should not inject the interrupt. // device should not inject the interrupt.
// Instead, the Pending Bit Array table is updated to reflect there // Instead, the Pending Bit Array table is updated to reflect there
// is a pending interrupt for this specific vector. // is a pending interrupt for this specific vector.
if config.is_masked() || entry.is_masked() { if config.is_masked() || entry.is_masked() {
config.set_pba_bit(queue.vector, false); config.set_pba_bit(queue.vector, false);
return Ok(()); return Ok(());
} }
(msi_cb)(InterruptParameters { msix: Some(entry) }) (msi_cb)(InterruptParameters { msix: Some(entry) })
}) as VirtioInterrupt); }) as VirtioInterrupt);
self.interrupt_cb = Some(cb); self.interrupt_cb = Some(cb);
}
} }
fn ioeventfds(&self) -> Vec<(&EventFd, u64, u64)> { fn ioeventfds(&self) -> Vec<(&EventFd, u64, u64)> {
@ -470,16 +480,20 @@ impl PciDevice for VirtioPciDevice {
// Handled with ioeventfds. // Handled with ioeventfds.
} }
o if MSIX_TABLE_BAR_OFFSET <= o && o < MSIX_TABLE_BAR_OFFSET + MSIX_TABLE_SIZE => { o if MSIX_TABLE_BAR_OFFSET <= o && o < MSIX_TABLE_BAR_OFFSET + MSIX_TABLE_SIZE => {
self.msix_config if let Some(msix_config) = &self.msix_config {
.lock() msix_config
.unwrap() .lock()
.read_table(o - MSIX_TABLE_BAR_OFFSET, data); .unwrap()
.read_table(o - MSIX_TABLE_BAR_OFFSET, data);
}
} }
o if MSIX_PBA_BAR_OFFSET <= o && o < MSIX_PBA_BAR_OFFSET + MSIX_PBA_SIZE => { o if MSIX_PBA_BAR_OFFSET <= o && o < MSIX_PBA_BAR_OFFSET + MSIX_PBA_SIZE => {
self.msix_config if let Some(msix_config) = &self.msix_config {
.lock() msix_config
.unwrap() .lock()
.read_pba(o - MSIX_PBA_BAR_OFFSET, data); .unwrap()
.read_pba(o - MSIX_PBA_BAR_OFFSET, data);
}
} }
_ => (), _ => (),
} }
@ -510,16 +524,20 @@ impl PciDevice for VirtioPciDevice {
// Handled with ioeventfds. // Handled with ioeventfds.
} }
o if MSIX_TABLE_BAR_OFFSET <= o && o < MSIX_TABLE_BAR_OFFSET + MSIX_TABLE_SIZE => { o if MSIX_TABLE_BAR_OFFSET <= o && o < MSIX_TABLE_BAR_OFFSET + MSIX_TABLE_SIZE => {
self.msix_config if let Some(msix_config) = &self.msix_config {
.lock() msix_config
.unwrap() .lock()
.write_table(o - MSIX_TABLE_BAR_OFFSET, data); .unwrap()
.write_table(o - MSIX_TABLE_BAR_OFFSET, data);
}
} }
o if MSIX_PBA_BAR_OFFSET <= o && o < MSIX_PBA_BAR_OFFSET + MSIX_PBA_SIZE => { o if MSIX_PBA_BAR_OFFSET <= o && o < MSIX_PBA_BAR_OFFSET + MSIX_PBA_SIZE => {
self.msix_config if let Some(msix_config) = &self.msix_config {
.lock() msix_config
.unwrap() .lock()
.write_pba(o - MSIX_PBA_BAR_OFFSET, data); .unwrap()
.write_pba(o - MSIX_PBA_BAR_OFFSET, data);
}
} }
_ => (), _ => (),
}; };

View File

@ -429,9 +429,10 @@ impl DeviceManager {
mmio_bus: &mut devices::Bus, mmio_bus: &mut devices::Bus,
msi_capable: bool, msi_capable: bool,
) -> DeviceManagerResult<()> { ) -> DeviceManagerResult<()> {
let mut virtio_pci_device = let msix_num = if msi_capable { DEFAULT_MSIX_VEC_NUM } else { 0 };
VirtioPciDevice::new(memory, virtio_device, DEFAULT_MSIX_VEC_NUM)
.map_err(DeviceManagerError::VirtioDevice)?; let mut virtio_pci_device = VirtioPciDevice::new(memory, virtio_device, msix_num)
.map_err(DeviceManagerError::VirtioDevice)?;
let bars = virtio_pci_device let bars = virtio_pci_device
.allocate_bars(allocator) .allocate_bars(allocator)