From f67785e9e42c5b2bd11d882b79bbbcab97fceeba Mon Sep 17 00:00:00 2001 From: Xiong Zhang Date: Thu, 20 May 2021 19:10:26 +0800 Subject: [PATCH] devices:pcie: Specify Bus number at pci bridge creation When pci bridige is created, caller should specify its primary bus number and secondary bus number. And supposing kernel won't modify them as the current pci device topology is simple enough, once guest modify them, warn message will be printed. The secondary bus number shouldn't be assigned to any other device, so this patch loop the bus number to find a free bus number. BUG=b:185084350 TEST=Boot a vm with passthrough device and check its function Change-Id: Iae72a0e0401a6e75c62582456b92792a1a36211a Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2955569 Tested-by: kokoro Commit-Queue: Daniel Verkamp Reviewed-by: Daniel Verkamp --- devices/src/pci/pcie.rs | 57 +++++++++++++++++++++++++++++-- resources/src/system_allocator.rs | 9 +++++ src/linux.rs | 15 ++++++-- 3 files changed, 76 insertions(+), 5 deletions(-) diff --git a/devices/src/pci/pcie.rs b/devices/src/pci/pcie.rs index fc1026c206..35f601f229 100644 --- a/devices/src/pci/pcie.rs +++ b/devices/src/pci/pcie.rs @@ -32,10 +32,14 @@ pub trait PcieDevice: Send { const BR_MSIX_TABLE_OFFSET: u64 = 0x0; const BR_MSIX_PBA_OFFSET: u64 = 0x100; const PCI_BRIDGE_BAR_SIZE: u64 = 0x1000; +const BR_BUS_NUMBER_REG: usize = 0x6; pub struct PciBridge { device: Box, config: PciConfiguration, pci_address: Option, + primary_number: u8, + secondary_number: u8, + subordinate_number: u8, setting_bar: u8, msix_config: Arc>, msix_cap_reg_idx: Option, @@ -44,10 +48,15 @@ pub struct PciBridge { } impl PciBridge { - pub fn new(device: Box, msi_device_tube: Tube) -> Self { + pub fn new( + device: Box, + msi_device_tube: Tube, + primary_number: u8, + secondary_number: u8, + ) -> Self { let msix_config = Arc::new(Mutex::new(MsixConfig::new(1, msi_device_tube))); let device_id = device.get_device_id(); - let config = PciConfiguration::new( + let mut config = PciConfiguration::new( PCI_VENDOR_ID_INTEL, device_id, PciClassCode::BridgeDevice, @@ -60,10 +69,16 @@ impl PciBridge { 0, ); + let data = [primary_number, secondary_number, secondary_number, 0]; + config.write_reg(BR_BUS_NUMBER_REG, 0, &data[..]); + PciBridge { device, config, pci_address: None, + primary_number, + secondary_number, + subordinate_number: secondary_number, setting_bar: 0, msix_config, msix_cap_reg_idx: None, @@ -83,7 +98,8 @@ impl PciDevice for PciBridge { resources: &mut SystemAllocator, ) -> std::result::Result { if self.pci_address.is_none() { - self.pci_address = match resources.allocate_pci(0, self.debug_label()) { + self.pci_address = match resources.allocate_pci(self.primary_number, self.debug_label()) + { Some(Alloc::PciBar { bus, dev, @@ -227,6 +243,41 @@ impl PciDevice for PciBridge { } } + // Suppose kernel won't modify primary/secondary/subordinate bus number, + // if it indeed modify, print a warning + if reg_idx == BR_BUS_NUMBER_REG { + let len = data.len(); + if offset == 0 && len == 1 && data[0] != self.primary_number { + warn!( + "kernel modify primary bus number: {} -> {}", + self.primary_number, data[0] + ); + } else if offset == 0 && len == 2 { + if data[0] != self.primary_number { + warn!( + "kernel modify primary bus number: {} -> {}", + self.primary_number, data[0] + ); + } + if data[1] != self.secondary_number { + warn!( + "kernel modify secondary bus number: {} -> {}", + self.secondary_number, data[1] + ); + } + } else if offset == 1 && len == 1 && data[0] != self.secondary_number { + warn!( + "kernel modify secondary bus number: {} -> {}", + self.secondary_number, data[0] + ); + } else if offset == 2 && len == 1 && data[0] != self.subordinate_number { + warn!( + "kernel modify subordinate bus number: {} -> {}", + self.subordinate_number, data[0] + ); + } + } + self.device.write_config(reg_idx, offset, data); (&mut self.config).write_reg(reg_idx, offset, data) diff --git a/resources/src/system_allocator.rs b/resources/src/system_allocator.rs index fdccc4e5dc..688dc430fc 100644 --- a/resources/src/system_allocator.rs +++ b/resources/src/system_allocator.rs @@ -153,6 +153,15 @@ impl SystemAllocator { self.pci_allocator.get_mut(&bus) } + // Check whether devices exist or not on the specified bus + pub fn pci_bus_empty(&self, bus: u8) -> bool { + if self.pci_allocator.get(&bus).is_none() { + true + } else { + false + } + } + /// Allocate PCI slot location. pub fn allocate_pci(&mut self, bus: u8, tag: String) -> Option { let id = self.get_anon_alloc(); diff --git a/src/linux.rs b/src/linux.rs index d7916dca8c..e6fc06c78d 100644 --- a/src/linux.rs +++ b/src/linux.rs @@ -51,8 +51,8 @@ use devices::Ac97Dev; use devices::ProtectionType; use devices::{ self, BusDeviceObj, HostHotPlugKey, IrqChip, IrqEventIndex, KvmKernelIrqChip, PciAddress, - PciDevice, StubPciDevice, VcpuRunState, VfioContainer, VfioDevice, VfioPciDevice, - VfioPlatformDevice, VirtioPciDevice, + PciBridge, PciDevice, PcieRootPort, StubPciDevice, VcpuRunState, VfioContainer, VfioDevice, + VfioPciDevice, VfioPlatformDevice, VirtioPciDevice, }; #[cfg(feature = "usb")] use devices::{HostBackendDeviceProvider, XhciController}; @@ -1801,6 +1801,17 @@ fn create_devices( } } + // Create Pcie Root Port + let pcie_root_port = Box::new(PcieRootPort::new()); + let (msi_host_tube, msi_device_tube) = Tube::pair().context("failed to create tube")?; + control_tubes.push(TaggedControlTube::VmIrq(msi_host_tube)); + let sec_bus = (1..255) + .find(|&bus_num| resources.pci_bus_empty(bus_num)) + .context("failed to find empty bus for Pci hotplug")?; + let pci_bridge = Box::new(PciBridge::new(pcie_root_port, msi_device_tube, 0, sec_bus)); + // pcie root port is used in hotplug process only, so disable sandbox for it + devices.push((pci_bridge, None)); + for params in &cfg.stub_pci_devices { // Stub devices don't need jailing since they don't do anything. devices.push((Box::new(StubPciDevice::new(params)), None));