arch: ensure only one IRQ is used per PCI device-pin combination

Previously, the IRQ allocation logic could allocate different IRQ
numbers for the same pin (INTA#/INTB#/...) within the same device. This
would result in conflicting MPTable IRQ source entries where the bus,
device, and pin were the same but the IRQ number was different.

This was not apparent in practice in most cases, since almost all PCI
devices in crosvm support MSI-X, so the pin-based interrupts were not
used. However, the xhci device does not support MSI/MSI-X, so it was
affected when it was allocated one of the conflicting IRQ numbers.

Now that the generate_pci_root code in arch chooses the pin numbers
before assigning IRQs, we can fix this problem by recording previously
used pin to irq mappings within each PCI (bus, device) and reusing them
when appropriate.

BUG=b:3834424
TEST=Attach USB device to ARCVM on hatch
TEST=Start crosvm with >32 PCI devices and `pci=nomsi`

Change-Id: Ie40bcc93e71fc494bff1c5fb2d916249099be87a
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3840307
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Vineeth Pillai <vineethrp@google.com>
Reviewed-by: Noah Gold <nkgold@google.com>
This commit is contained in:
Daniel Verkamp 2022-08-18 10:10:26 -07:00 committed by crosvm LUCI
parent 9846f3f040
commit 6d136c86b6
2 changed files with 27 additions and 10 deletions

View file

@ -692,7 +692,10 @@ pub fn generate_pci_root(
// Allocate legacy INTx
let mut pci_irqs = Vec::new();
let mut irqs: Vec<Option<u32>> = vec![None; max_irqs];
let mut irqs: Vec<u32> = Vec::new();
// Mapping of (bus, dev, pin) -> IRQ number.
let mut dev_pin_irq = BTreeMap::new();
for (dev_idx, (device, _jail)) in devices.iter_mut().enumerate() {
let pci_address = device_addrs[dev_idx];
@ -714,15 +717,29 @@ pub fn generate_pci_root(
_ => PciInterruptPin::IntD,
};
// For default interrupt routing use next preallocated interrupt from the pool.
let irq_num = if let Some(irq) = irqs[dev_idx % max_irqs] {
irq
// If an IRQ number has already been assigned for a different function with this (bus,
// device, pin) combination, use it. Otherwise allocate a new one and insert it into the
// map.
let pin_key = (pci_address.bus, pci_address.dev, pin);
let irq_num = if let Some(irq_num) = dev_pin_irq.get(&pin_key) {
*irq_num
} else {
let irq = resources
.allocate_irq()
.ok_or(DeviceRegistrationError::AllocateIrq)?;
irqs[dev_idx % max_irqs] = Some(irq);
irq
// If we have allocated fewer than `max_irqs` total, add a new irq to the `irqs`
// pool. Otherwise, share one of the existing `irqs`.
let irq_num = if irqs.len() < max_irqs {
let irq_num = resources
.allocate_irq()
.ok_or(DeviceRegistrationError::AllocateIrq)?;
irqs.push(irq_num);
irq_num
} else {
// Pick one of the existing IRQs to share, using `dev_idx` to distribute IRQ
// sharing evenly across devices.
irqs[dev_idx % max_irqs]
};
dev_pin_irq.insert(pin_key, irq_num);
irq_num
};
(pin, irq_num)
};

View file

@ -93,7 +93,7 @@ pub use self::stub::StubPciParameters;
pub use self::vfio_pci::VfioPciDevice;
/// PCI has four interrupt pins A->D.
#[derive(Copy, Clone)]
#[derive(Copy, Clone, Ord, PartialOrd, PartialEq, Eq)]
pub enum PciInterruptPin {
IntA,
IntB,