From 07bef815cc5613191b48fe1fca450627c264a873 Mon Sep 17 00:00:00 2001 From: Henry Wang Date: Thu, 25 Nov 2021 03:12:37 -0500 Subject: [PATCH] aarch64: Introduce struct `PciSpaceInfo` for FDT Currently, a tuple containing PCI space start address and PCI space size is used to pass the PCI space information to the FDT creator. In order to support the multiple PCI segment for FDT, more information such as the PCI segment ID should be passed to the FDT creator. If we still use a tuple to store these information, the code flexibility and readablity will be harmed. To address this issue, this commit replaces the tuple containing the PCI space information to a structure `PciSpaceInfo` and uses a vector of `PciSpaceInfo` to store PCI space information for each segment, so that multiple PCI segment information can be passed to the FDT together. Note that the scope of this commit will only contain the refactor of original code, the actual multiple PCI segments support will be in following series, and for now `--platform num_pci_segments` should only be 1. Signed-off-by: Henry Wang --- arch/src/aarch64/fdt.rs | 192 ++++++++++++++++++------------------- arch/src/aarch64/layout.rs | 2 +- arch/src/aarch64/mod.rs | 6 +- arch/src/lib.rs | 10 ++ vmm/src/device_manager.rs | 2 +- vmm/src/vm.rs | 42 ++++---- 6 files changed, 130 insertions(+), 124 deletions(-) diff --git a/arch/src/aarch64/fdt.rs b/arch/src/aarch64/fdt.rs index 77863b1c9..577f86dff 100644 --- a/arch/src/aarch64/fdt.rs +++ b/arch/src/aarch64/fdt.rs @@ -6,7 +6,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the THIRD-PARTY file. -use crate::NumaNodes; +use crate::{NumaNodes, PciSpaceInfo}; use byteorder::{BigEndian, ByteOrder}; use std::cmp; use std::collections::HashMap; @@ -88,7 +88,7 @@ pub fn create_fdt, gic_device: &dyn GicDevice, initrd: &Option, - pci_space_address: &(u64, u64), + pci_space_info: &[PciSpaceInfo], numa_nodes: &NumaNodes, virtio_iommu_bdf: Option, ) -> FdtWriterResult> { @@ -117,12 +117,7 @@ pub fn create_fdt 1 { create_distance_map_node(&mut fdt, numa_nodes)?; } @@ -547,108 +542,113 @@ fn create_devices_node, ) -> FdtWriterResult<()> { // Add node for PCIe controller. // See Documentation/devicetree/bindings/pci/host-generic-pci.txt in the kernel // and https://elinux.org/Device_Tree_Usage. + // In multiple PCI segments setup, each PCI segment needs a PCI node. + for pci_device_info_elem in pci_device_info.iter() { + // EDK2 requires the PCIe high space above 4G address. + // The actual space in CLH follows the RAM. If the RAM space is small, the PCIe high space + // could fall bellow 4G. + // Here we cut off PCI device space below 8G in FDT to workaround the EDK2 check. + // But the address written in ACPI is not impacted. + let (pci_device_base_64bit, pci_device_size_64bit) = if cfg!(feature = "acpi") + && (pci_device_info_elem.pci_device_space_start < PCI_HIGH_BASE) + { + ( + PCI_HIGH_BASE, + pci_device_info_elem.pci_device_space_size + - (PCI_HIGH_BASE - pci_device_info_elem.pci_device_space_start), + ) + } else { + ( + pci_device_info_elem.pci_device_space_start, + pci_device_info_elem.pci_device_space_size, + ) + }; - // EDK2 requires the PCIe high space above 4G address. - // The actual space in CLH follows the RAM. If the RAM space is small, the PCIe high space - // could fall bellow 4G. - // Here we put it above 512G in FDT to workaround the EDK2 check. - // But the address written in ACPI is not impacted. - let pci_device_base_64bit: u64 = if cfg!(feature = "acpi") { - pci_device_base + PCI_HIGH_BASE - } else { - pci_device_base - }; - let pci_device_size_64bit: u64 = if cfg!(feature = "acpi") { - pci_device_size - PCI_HIGH_BASE - } else { - pci_device_size - }; - - let ranges = [ - // io addresses - 0x1000000, - 0_u32, - 0_u32, - (MEM_PCI_IO_START.0 >> 32) as u32, - MEM_PCI_IO_START.0 as u32, - (MEM_PCI_IO_SIZE >> 32) as u32, - MEM_PCI_IO_SIZE as u32, - // mmio addresses - 0x2000000, // (ss = 10: 32-bit memory space) - (MEM_32BIT_DEVICES_START.0 >> 32) as u32, // PCI address - MEM_32BIT_DEVICES_START.0 as u32, - (MEM_32BIT_DEVICES_START.0 >> 32) as u32, // CPU address - MEM_32BIT_DEVICES_START.0 as u32, - (MEM_32BIT_DEVICES_SIZE >> 32) as u32, // size - MEM_32BIT_DEVICES_SIZE as u32, - // device addresses - 0x3000000, // (ss = 11: 64-bit memory space) - (pci_device_base_64bit >> 32) as u32, // PCI address - pci_device_base_64bit as u32, - (pci_device_base_64bit >> 32) as u32, // CPU address - pci_device_base_64bit as u32, - (pci_device_size_64bit >> 32) as u32, // size - pci_device_size_64bit as u32, - ]; - let bus_range = [0, 0]; // Only bus 0 - let reg = [PCI_MMCONFIG_START.0, PCI_MMCONFIG_SIZE]; - - let pci_node = fdt.begin_node("pci")?; - fdt.property_string("compatible", "pci-host-ecam-generic")?; - fdt.property_string("device_type", "pci")?; - fdt.property_array_u32("ranges", &ranges)?; - fdt.property_array_u32("bus-range", &bus_range)?; - fdt.property_u32("#address-cells", 3)?; - fdt.property_u32("#size-cells", 2)?; - fdt.property_array_u64("reg", ®)?; - fdt.property_u32("#interrupt-cells", 1)?; - fdt.property_null("interrupt-map")?; - fdt.property_null("interrupt-map-mask")?; - fdt.property_null("dma-coherent")?; - fdt.property_u32("msi-parent", MSI_PHANDLE)?; - - if let Some(virtio_iommu_bdf) = virtio_iommu_bdf { - // See kernel document Documentation/devicetree/bindings/pci/pci-iommu.txt - // for 'iommu-map' attribute setting. - let iommu_map = [ + let ranges = [ + // io addresses + 0x1000000, 0_u32, - VIRTIO_IOMMU_PHANDLE, 0_u32, - virtio_iommu_bdf, - virtio_iommu_bdf + 1, - VIRTIO_IOMMU_PHANDLE, - virtio_iommu_bdf + 1, - 0xffff - virtio_iommu_bdf, + (MEM_PCI_IO_START.0 >> 32) as u32, + MEM_PCI_IO_START.0 as u32, + (MEM_PCI_IO_SIZE >> 32) as u32, + MEM_PCI_IO_SIZE as u32, + // mmio addresses + 0x2000000, // (ss = 10: 32-bit memory space) + (MEM_32BIT_DEVICES_START.0 >> 32) as u32, // PCI address + MEM_32BIT_DEVICES_START.0 as u32, + (MEM_32BIT_DEVICES_START.0 >> 32) as u32, // CPU address + MEM_32BIT_DEVICES_START.0 as u32, + (MEM_32BIT_DEVICES_SIZE >> 32) as u32, // size + MEM_32BIT_DEVICES_SIZE as u32, + // device addresses + 0x3000000, // (ss = 11: 64-bit memory space) + (pci_device_base_64bit >> 32) as u32, // PCI address + pci_device_base_64bit as u32, + (pci_device_base_64bit >> 32) as u32, // CPU address + pci_device_base_64bit as u32, + (pci_device_size_64bit >> 32) as u32, // size + pci_device_size_64bit as u32, ]; - fdt.property_array_u32("iommu-map", &iommu_map)?; + let bus_range = [0, 0]; // Only bus 0 + let reg = [PCI_MMCONFIG_START.0, PCI_MMCONFIG_SIZE]; - // See kernel document Documentation/devicetree/bindings/virtio/iommu.txt - // for virtio-iommu node settings. - let virtio_iommu_node_name = format!("virtio_iommu@{:x}", virtio_iommu_bdf); - let virtio_iommu_node = fdt.begin_node(&virtio_iommu_node_name)?; - fdt.property_u32("#iommu-cells", 1)?; - fdt.property_string("compatible", "virtio,pci-iommu")?; + let pci_node = fdt.begin_node("pci")?; + fdt.property_string("compatible", "pci-host-ecam-generic")?; + fdt.property_string("device_type", "pci")?; + fdt.property_array_u32("ranges", &ranges)?; + fdt.property_array_u32("bus-range", &bus_range)?; + fdt.property_u32("#address-cells", 3)?; + fdt.property_u32("#size-cells", 2)?; + fdt.property_array_u64("reg", ®)?; + fdt.property_u32("#interrupt-cells", 1)?; + fdt.property_null("interrupt-map")?; + fdt.property_null("interrupt-map-mask")?; + fdt.property_null("dma-coherent")?; + fdt.property_u32("msi-parent", MSI_PHANDLE)?; - // 'reg' is a five-cell address encoded as - // (phys.hi phys.mid phys.lo size.hi size.lo). phys.hi should contain the - // device's BDF as 0b00000000 bbbbbbbb dddddfff 00000000. The other cells - // should be zero. - let reg = [virtio_iommu_bdf << 8, 0_u32, 0_u32, 0_u32, 0_u32]; - fdt.property_array_u32("reg", ®)?; - fdt.property_u32("phandle", VIRTIO_IOMMU_PHANDLE)?; + if let Some(virtio_iommu_bdf) = virtio_iommu_bdf { + // See kernel document Documentation/devicetree/bindings/pci/pci-iommu.txt + // for 'iommu-map' attribute setting. + let iommu_map = [ + 0_u32, + VIRTIO_IOMMU_PHANDLE, + 0_u32, + virtio_iommu_bdf, + virtio_iommu_bdf + 1, + VIRTIO_IOMMU_PHANDLE, + virtio_iommu_bdf + 1, + 0xffff - virtio_iommu_bdf, + ]; + fdt.property_array_u32("iommu-map", &iommu_map)?; - fdt.end_node(virtio_iommu_node)?; + // See kernel document Documentation/devicetree/bindings/virtio/iommu.txt + // for virtio-iommu node settings. + let virtio_iommu_node_name = format!("virtio_iommu@{:x}", virtio_iommu_bdf); + let virtio_iommu_node = fdt.begin_node(&virtio_iommu_node_name)?; + fdt.property_u32("#iommu-cells", 1)?; + fdt.property_string("compatible", "virtio,pci-iommu")?; + + // 'reg' is a five-cell address encoded as + // (phys.hi phys.mid phys.lo size.hi size.lo). phys.hi should contain the + // device's BDF as 0b00000000 bbbbbbbb dddddfff 00000000. The other cells + // should be zero. + let reg = [virtio_iommu_bdf << 8, 0_u32, 0_u32, 0_u32, 0_u32]; + fdt.property_array_u32("reg", ®)?; + fdt.property_u32("phandle", VIRTIO_IOMMU_PHANDLE)?; + + fdt.end_node(virtio_iommu_node)?; + } + + fdt.end_node(pci_node)?; } - fdt.end_node(pci_node)?; - Ok(()) } diff --git a/arch/src/aarch64/layout.rs b/arch/src/aarch64/layout.rs index f0f17fe5b..b464b592e 100644 --- a/arch/src/aarch64/layout.rs +++ b/arch/src/aarch64/layout.rs @@ -104,7 +104,7 @@ pub const RSDP_POINTER: GuestAddress = GuestAddress(ACPI_START); pub const KERNEL_START: u64 = ACPI_START + ACPI_MAX_SIZE as u64; /// Pci high memory base -pub const PCI_HIGH_BASE: u64 = 0x80_0000_0000_u64; +pub const PCI_HIGH_BASE: u64 = 0x2_0000_0000_u64; // As per virt/kvm/arm/vgic/vgic-kvm-device.c we need // the number of interrupts our GIC will support to be: diff --git a/arch/src/aarch64/mod.rs b/arch/src/aarch64/mod.rs index 3054c68b7..0a915d423 100644 --- a/arch/src/aarch64/mod.rs +++ b/arch/src/aarch64/mod.rs @@ -14,7 +14,7 @@ pub mod regs; pub mod uefi; pub use self::fdt::DeviceInfoForFdt; -use crate::{DeviceType, GuestMemoryMmap, NumaNodes, RegionType}; +use crate::{DeviceType, GuestMemoryMmap, NumaNodes, PciSpaceInfo, RegionType}; use gic::GicDevice; use log::{log_enabled, Level}; use std::collections::HashMap; @@ -140,7 +140,7 @@ pub fn configure_system, device_info: &HashMap<(DeviceType, String), T, S>, initrd: &Option, - pci_space_address: &(u64, u64), + pci_space_info: &[PciSpaceInfo], virtio_iommu_bdf: Option, gic_device: &dyn GicDevice, numa_nodes: &NumaNodes, @@ -153,7 +153,7 @@ pub fn configure_system u64 { diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index 0e681cedc..27910b60e 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -3407,7 +3407,7 @@ impl DeviceManager { Arc::clone(self.pci_segments[0].pci_config_io.as_ref().unwrap()) } - #[cfg(feature = "acpi")] + #[cfg(any(target_arch = "aarch64", feature = "acpi"))] pub(crate) fn pci_segments(&self) -> &Vec { &self.pci_segments } diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs index 604b40ef9..987d89251 100644 --- a/vmm/src/vm.rs +++ b/vmm/src/vm.rs @@ -38,6 +38,8 @@ use arch::x86_64::tdx::TdVmmDataRegionType; #[cfg(feature = "tdx")] use arch::x86_64::tdx::{TdVmmDataRegion, TdvfSection}; use arch::EntryPoint; +#[cfg(target_arch = "aarch64")] +use arch::PciSpaceInfo; #[cfg(any(target_arch = "aarch64", feature = "acpi"))] use arch::{NumaNode, NumaNodes}; use devices::AcpiNotificationFlags; @@ -72,7 +74,9 @@ use std::{result, str, thread}; use vm_device::Bus; #[cfg(target_arch = "x86_64")] use vm_device::BusDevice; -use vm_memory::{Address, Bytes, GuestAddress, GuestAddressSpace, GuestMemoryAtomic}; +#[cfg(target_arch = "x86_64")] +use vm_memory::Address; +use vm_memory::{Bytes, GuestAddress, GuestAddressSpace, GuestMemoryAtomic}; #[cfg(feature = "tdx")] use vm_memory::{GuestMemory, GuestMemoryRegion}; use vm_migration::{ @@ -1109,6 +1113,7 @@ impl Vm { let vcpu_mpidrs = self.cpu_manager.lock().unwrap().get_mpidrs(); let vcpu_topology = self.cpu_manager.lock().unwrap().get_vcpu_topology(); let mem = self.memory_manager.lock().unwrap().boot_guest_memory(); + let mut pci_space_info: Vec = Vec::new(); let initramfs_config = match self.initramfs { Some(_) => Some(self.load_initramfs(&mem)?), None => None, @@ -1121,26 +1126,17 @@ impl Vm { .get_device_info() .clone(); - let pci_space_start: GuestAddress = self - .memory_manager - .lock() - .as_ref() - .unwrap() - .start_of_device_area(); - - let pci_space_end: GuestAddress = self - .memory_manager - .lock() - .as_ref() - .unwrap() - .end_of_device_area(); - - let pci_space_size = pci_space_end - .checked_offset_from(pci_space_start) - .ok_or(Error::MemOverflow)? - + 1; - - let pci_space = (pci_space_start.0, pci_space_size); + for pci_segment in self.device_manager.lock().unwrap().pci_segments().iter() { + let pci_space = PciSpaceInfo { + pci_segment_id: pci_segment.id, + mmio_config_address: pci_segment.mmio_config_address, + pci_device_space_start: pci_segment.start_of_device_area, + pci_device_space_size: pci_segment.end_of_device_area + - pci_segment.start_of_device_area + + 1, + }; + pci_space_info.push(pci_space); + } let virtio_iommu_bdf = self .device_manager @@ -1165,7 +1161,7 @@ impl Vm { vcpu_topology, device_info, &initramfs_config, - &pci_space, + &pci_space_info, virtio_iommu_bdf.map(|bdf| bdf.into()), &*gic_device, &self.numa_nodes, @@ -2765,7 +2761,7 @@ mod tests { &dev_info, &*gic, &None, - &(0x1_0000_0000, 0x1_0000), + &Vec::new(), &BTreeMap::new(), None, )