pci: refactor FDT/MPTABLE creation to use PciAddress.

Simple refactor of FDT and MPTables generation to use PCI device
addressing and allow declatation of non-zero PCI bus ids for x86
architectures. It also allows non sequential IRQ allocation for
PCI devices.

BUG=None
TEST=build_test & tast run crostini.Sanity

Change-Id: I6cc31ce412199a732499b2d8d18d99f08d765690
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2175739
Tested-by: Tomasz Jeznach <tjeznach@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Tomasz Jeznach <tjeznach@chromium.org>

6 files changed, 74 insertions, 59 deletions

diff --git a/aarch64/src/fdt.rs b/aarch64/src/fdt.rs
index f50286e..fc976ef 100644
--- a/aarch64/src/fdt.rs
+++ b/aarch64/src/fdt.rs
@@ -11,7 +11,7 @@ use arch::fdt::{
     property_null, property_string, property_u32, property_u64, start_fdt, Error, Result,
 };
 use arch::SERIAL_ADDR;
-use devices::PciInterruptPin;
+use devices::{PciAddress, PciInterruptPin};
 use sys_util::{GuestAddress, GuestMemory};
 
 // This is the start of DRAM in the physical address space.
@@ -37,7 +37,6 @@ use crate::AARCH64_SERIAL_SIZE;
 use crate::AARCH64_SERIAL_SPEED;
 
 // These are related to guest virtio devices.
-use crate::AARCH64_IRQ_BASE;
 use crate::AARCH64_MMIO_BASE;
 use crate::AARCH64_MMIO_SIZE;
 use crate::AARCH64_PCI_CFG_BASE;
@@ -217,7 +216,7 @@ fn create_chosen_node(
 
 fn create_pci_nodes(
     fdt: &mut Vec<u8>,
-    pci_irqs: Vec<(u32, PciInterruptPin)>,
+    pci_irqs: Vec<(PciAddress, u32, PciInterruptPin)>,
     pci_device_base: u64,
     pci_device_size: u64,
 ) -> Result<()> {
@@ -248,14 +247,14 @@ fn create_pci_nodes(
     let mut interrupts: Vec<u32> = Vec::new();
     let mut masks: Vec<u32> = Vec::new();
 
-    for (i, pci_irq) in pci_irqs.iter().enumerate() {
+    for (address, irq_num, irq_pin) in pci_irqs.iter() {
         // PCI_DEVICE(3)
-        interrupts.push((pci_irq.0 + 1) << 11);
+        interrupts.push(address.to_config_address(0));
         interrupts.push(0);
         interrupts.push(0);
 
         // INT#(1)
-        interrupts.push(pci_irq.1.to_mask() + 1);
+        interrupts.push(irq_pin.to_mask() + 1);
 
         // CONTROLLER(PHANDLE)
         interrupts.push(PHANDLE_GIC);
@@ -264,7 +263,7 @@ fn create_pci_nodes(
 
         // CONTROLLER_DATA(3)
         interrupts.push(GIC_FDT_IRQ_TYPE_SPI);
-        interrupts.push(AARCH64_IRQ_BASE + i as u32);
+        interrupts.push(*irq_num);
         interrupts.push(IRQ_TYPE_LEVEL_HIGH);
 
         // PCI_DEVICE(3)
@@ -331,7 +330,7 @@ fn create_rtc_node(fdt: &mut Vec<u8>) -> Result<()> {
 ///
 /// * `fdt_max_size` - The amount of space reserved for the device tree
 /// * `guest_mem` - The guest memory object
-/// * `pci_irqs` - List of PCI device number to PCI interrupt pin mappings
+/// * `pci_irqs` - List of PCI device address to PCI interrupt number and pin mappings
 /// * `num_cpus` - Number of virtual CPUs the guest will have
 /// * `fdt_load_offset` - The offset into physical memory for the device tree
 /// * `pci_device_base` - The offset into physical memory for PCI device memory
@@ -343,7 +342,7 @@ fn create_rtc_node(fdt: &mut Vec<u8>) -> Result<()> {
 pub fn create_fdt(
     fdt_max_size: usize,
     guest_mem: &GuestMemory,
-    pci_irqs: Vec<(u32, PciInterruptPin)>,
+    pci_irqs: Vec<(PciAddress, u32, PciInterruptPin)>,
     num_cpus: u32,
     fdt_load_offset: u64,
     pci_device_base: u64,
diff --git a/aarch64/src/lib.rs b/aarch64/src/lib.rs
index 1116b69..4c8af6b 100644
--- a/aarch64/src/lib.rs
+++ b/aarch64/src/lib.rs
@@ -15,7 +15,7 @@ use arch::{
     get_serial_cmdline, GetSerialCmdlineError, RunnableLinuxVm, SerialHardware, SerialParameters,
     VmComponents, VmImage,
 };
-use devices::{Bus, BusError, PciConfigMmio, PciDevice, PciInterruptPin};
+use devices::{Bus, BusError, PciAddress, PciConfigMmio, PciDevice, PciInterruptPin};
 use io_jail::Minijail;
 use remain::sorted;
 use resources::SystemAllocator;
@@ -342,7 +342,7 @@ impl AArch64 {
         vcpu_count: u32,
         cmdline: &CStr,
         initrd_file: Option<File>,
-        pci_irqs: Vec<(u32, PciInterruptPin)>,
+        pci_irqs: Vec<(PciAddress, u32, PciInterruptPin)>,
         android_fstab: Option<File>,
         kernel_end: u64,
         is_gicv3: bool,
diff --git a/arch/src/lib.rs b/arch/src/lib.rs
index 6fedf9d..ba09fa2 100644
--- a/arch/src/lib.rs
+++ b/arch/src/lib.rs
@@ -184,8 +184,14 @@ pub fn generate_pci_root(
     mmio_bus: &mut Bus,
     resources: &mut SystemAllocator,
     vm: &mut Vm,
-) -> Result<(PciRoot, Vec<(u32, PciInterruptPin)>, BTreeMap<u32, String>), DeviceRegistrationError>
-{
+) -> Result<
+    (
+        PciRoot,
+        Vec<(PciAddress, u32, PciInterruptPin)>,
+        BTreeMap<u32, String>,
+    ),
+    DeviceRegistrationError,
+> {
     let mut root = PciRoot::new();
     let mut pci_irqs = Vec::new();
     let mut pid_labels = BTreeMap::new();
@@ -230,7 +236,7 @@ pub fn generate_pci_root(
         keep_fds.push(irqfd.as_raw_fd());
         keep_fds.push(irq_resample_fd.as_raw_fd());
         device.assign_irq(irqfd, irq_resample_fd, irq_num, pci_irq_pin);
-        pci_irqs.push((dev_idx as u32, pci_irq_pin));
+        pci_irqs.push((address, irq_num, pci_irq_pin));
 
         let ranges = device
             .allocate_io_bars(resources)
diff --git a/devices/src/pci/pci_root.rs b/devices/src/pci/pci_root.rs
index 4f0e453..91d6094 100644
--- a/devices/src/pci/pci_root.rs
+++ b/devices/src/pci/pci_root.rs
@@ -56,6 +56,33 @@ impl Display for PciAddress {
 }
 
 impl PciAddress {
+    const BUS_OFFSET: usize = 16;
+    const BUS_MASK: u32 = 0x00ff;
+    const DEVICE_OFFSET: usize = 11;
+    const DEVICE_MASK: u32 = 0x1f;
+    const FUNCTION_OFFSET: usize = 8;
+    const FUNCTION_MASK: u32 = 0x07;
+    const REGISTER_OFFSET: usize = 2;
+    const REGISTER_MASK: u32 = 0x3f;
+
+    /// Construct PciAddress and register tuple from CONFIG_ADDRESS value.
+    pub fn from_config_address(config_address: u32) -> (Self, usize) {
+        let bus = ((config_address >> Self::BUS_OFFSET) & Self::BUS_MASK) as u8;
+        let dev = ((config_address >> Self::DEVICE_OFFSET) & Self::DEVICE_MASK) as u8;
+        let func = ((config_address >> Self::FUNCTION_OFFSET) & Self::FUNCTION_MASK) as u8;
+        let register = ((config_address >> Self::REGISTER_OFFSET) & Self::REGISTER_MASK) as usize;
+
+        (PciAddress { bus, dev, func }, register)
+    }
+
+    /// Encode PciAddress into CONFIG_ADDRESS value.
+    pub fn to_config_address(&self, register: usize) -> u32 {
+        ((Self::BUS_MASK & self.bus as u32) << Self::BUS_OFFSET)
+            | ((Self::DEVICE_MASK & self.dev as u32) << Self::DEVICE_OFFSET)
+            | ((Self::FUNCTION_MASK & self.func as u32) << Self::FUNCTION_OFFSET)
+            | ((Self::REGISTER_MASK & register as u32) << Self::REGISTER_OFFSET)
+    }
+
     /// Returns true if the address points to PCI root host-bridge.
     fn is_root(&self) -> bool {
         matches!(
@@ -159,7 +186,7 @@ impl PciConfigIo {
             return 0xffff_ffff;
         }
 
-        let (address, register) = parse_config_address(self.config_address & !0x8000_0000);
+        let (address, register) = PciAddress::from_config_address(self.config_address);
         self.pci_root.config_space_read(address, register)
     }
 
@@ -169,7 +196,7 @@ impl PciConfigIo {
             return;
         }
 
-        let (address, register) = parse_config_address(self.config_address & !0x8000_0000);
+        let (address, register) = PciAddress::from_config_address(self.config_address);
         self.pci_root
             .config_space_write(address, register, offset, data)
     }
@@ -243,12 +270,12 @@ impl PciConfigMmio {
     }
 
     fn config_space_read(&self, config_address: u32) -> u32 {
-        let (address, register) = parse_config_address(config_address);
+        let (address, register) = PciAddress::from_config_address(config_address);
         self.pci_root.config_space_read(address, register)
     }
 
     fn config_space_write(&mut self, config_address: u32, offset: u64, data: &[u8]) {
-        let (address, register) = parse_config_address(config_address);
+        let (address, register) = PciAddress::from_config_address(config_address);
         self.pci_root
             .config_space_write(address, register, offset, data)
     }
@@ -283,23 +310,3 @@ impl BusDevice for PciConfigMmio {
         self.config_space_write(offset as u32, offset % 4, data)
     }
 }
-
-// Parse the CONFIG_ADDRESS register to a (bus, device, function, register) tuple.
-fn parse_config_address(config_address: u32) -> (PciAddress, usize) {
-    const BUS_NUMBER_OFFSET: usize = 16;
-    const BUS_NUMBER_MASK: u32 = 0x00ff;
-    const DEVICE_NUMBER_OFFSET: usize = 11;
-    const DEVICE_NUMBER_MASK: u32 = 0x1f;
-    const FUNCTION_NUMBER_OFFSET: usize = 8;
-    const FUNCTION_NUMBER_MASK: u32 = 0x07;
-    const REGISTER_NUMBER_OFFSET: usize = 2;
-    const REGISTER_NUMBER_MASK: u32 = 0x3f;
-
-    let bus = ((config_address >> BUS_NUMBER_OFFSET) & BUS_NUMBER_MASK) as u8;
-    let dev = ((config_address >> DEVICE_NUMBER_OFFSET) & DEVICE_NUMBER_MASK) as u8;
-    let func = ((config_address >> FUNCTION_NUMBER_OFFSET) & FUNCTION_NUMBER_MASK) as u8;
-    let register_number =
-        ((config_address >> REGISTER_NUMBER_OFFSET) & REGISTER_NUMBER_MASK) as usize;
-
-    (PciAddress { bus, dev, func }, register_number)
-}
diff --git a/x86_64/src/lib.rs b/x86_64/src/lib.rs
index 2e54998..a6a02bc 100644
--- a/x86_64/src/lib.rs
+++ b/x86_64/src/lib.rs
@@ -60,7 +60,7 @@ use arch::{
 };
 use devices::split_irqchip_common::GsiRelay;
 use devices::{
-    Ioapic, PciConfigIo, PciDevice, PciInterruptPin, Pic, IOAPIC_BASE_ADDRESS,
+    Ioapic, PciAddress, PciConfigIo, PciDevice, PciInterruptPin, Pic, IOAPIC_BASE_ADDRESS,
     IOAPIC_MEM_LENGTH_BYTES,
 };
 use io_jail::Minijail;
@@ -212,7 +212,7 @@ fn configure_system(
     cmdline_addr: GuestAddress,
     cmdline_size: usize,
     num_cpus: u8,
-    pci_irqs: Vec<(u32, PciInterruptPin)>,
+    pci_irqs: Vec<(PciAddress, u32, PciInterruptPin)>,
     setup_data: Option<GuestAddress>,
     initrd: Option<(GuestAddress, usize)>,
     mut params: boot_params,
@@ -588,7 +588,7 @@ impl X8664arch {
         vcpu_count: u32,
         cmdline: &CStr,
         initrd_file: Option<File>,
-        pci_irqs: Vec<(u32, PciInterruptPin)>,
+        pci_irqs: Vec<(PciAddress, u32, PciInterruptPin)>,
         android_fstab: Option<File>,
         kernel_end: u64,
         params: boot_params,
diff --git a/x86_64/src/mptable.rs b/x86_64/src/mptable.rs
index cac9e58..218f561 100644
--- a/x86_64/src/mptable.rs
+++ b/x86_64/src/mptable.rs
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+use std::convert::TryFrom;
 use std::fmt::{self, Display};
 use std::mem;
 use std::result;
@@ -10,7 +11,7 @@ use std::slice;
 use libc::c_char;
 
 use data_model::VolatileMemory;
-use devices::PciInterruptPin;
+use devices::{PciAddress, PciInterruptPin};
 use sys_util::{GuestAddress, GuestMemory};
 
 use crate::mpspec::*;
@@ -117,14 +118,16 @@ fn compute_mp_size(num_cpus: u8) -> usize {
 pub fn setup_mptable(
     mem: &GuestMemory,
     num_cpus: u8,
-    pci_irqs: Vec<(u32, PciInterruptPin)>,
+    pci_irqs: Vec<(PciAddress, u32, PciInterruptPin)>,
 ) -> Result<()> {
-    const PCI_BUS_ID: u8 = 0;
-    const ISA_BUS_ID: u8 = 1;
-
     // Used to keep track of the next base pointer into the MP table.
     let mut base_mp = GuestAddress(MPTABLE_START);
 
+    // Calculate ISA bus number in the system, report at least one PCI bus.
+    let isa_bus_id = match pci_irqs.iter().max_by_key(|v| v.0.bus) {
+        Some(pci_irq) => pci_irq.0.bus + 1,
+        _ => 1,
+    };
     let mp_size = compute_mp_size(num_cpus);
 
     // The checked_add here ensures the all of the following base_mp.unchecked_add's will be without
@@ -194,11 +197,11 @@ pub fn setup_mptable(
         base_mp = base_mp.unchecked_add(size as u64);
         checksum = checksum.wrapping_add(compute_checksum(&mpc_ioapic));
     }
-    {
+    for pci_bus_id in 0..isa_bus_id {
         let size = mem::size_of::<mpc_bus>();
         let mut mpc_bus = mpc_bus::default();
         mpc_bus.type_ = MP_BUS as u8;
-        mpc_bus.busid = PCI_BUS_ID;
+        mpc_bus.busid = pci_bus_id;
         mpc_bus.bustype = BUS_TYPE_PCI;
         mem.write_obj_at_addr(mpc_bus, base_mp)
             .map_err(|_| Error::WriteMpcBus)?;
@@ -209,7 +212,7 @@ pub fn setup_mptable(
         let size = mem::size_of::<mpc_bus>();
         let mut mpc_bus = mpc_bus::default();
         mpc_bus.type_ = MP_BUS as u8;
-        mpc_bus.busid = ISA_BUS_ID;
+        mpc_bus.busid = isa_bus_id;
         mpc_bus.bustype = BUS_TYPE_ISA;
         mem.write_obj_at_addr(mpc_bus, base_mp)
             .map_err(|_| Error::WriteMpcBus)?;
@@ -222,7 +225,7 @@ pub fn setup_mptable(
         mpc_intsrc.type_ = MP_INTSRC as u8;
         mpc_intsrc.irqtype = mp_irq_source_types_mp_INT as u8;
         mpc_intsrc.irqflag = MP_IRQDIR_DEFAULT as u16;
-        mpc_intsrc.srcbus = ISA_BUS_ID;
+        mpc_intsrc.srcbus = isa_bus_id;
         mpc_intsrc.srcbusirq = 0;
         mpc_intsrc.dstapic = 0;
         mpc_intsrc.dstirq = 0;
@@ -239,7 +242,7 @@ pub fn setup_mptable(
         mpc_intsrc.type_ = MP_INTSRC as u8;
         mpc_intsrc.irqtype = mp_irq_source_types_mp_INT as u8;
         mpc_intsrc.irqflag = MP_IRQDIR_DEFAULT as u16;
-        mpc_intsrc.srcbus = ISA_BUS_ID;
+        mpc_intsrc.srcbus = isa_bus_id;
         mpc_intsrc.srcbusirq = i;
         mpc_intsrc.dstapic = ioapicid;
         mpc_intsrc.dstirq = i;
@@ -257,7 +260,7 @@ pub fn setup_mptable(
         mpc_intsrc.type_ = MP_INTSRC as u8;
         mpc_intsrc.irqtype = mp_irq_source_types_mp_INT as u8;
         mpc_intsrc.irqflag = (MP_IRQDIR_HIGH | MP_LEVEL_TRIGGER) as u16;
-        mpc_intsrc.srcbus = ISA_BUS_ID;
+        mpc_intsrc.srcbus = isa_bus_id;
         mpc_intsrc.srcbusirq = sci_irq;
         mpc_intsrc.dstapic = ioapicid;
         mpc_intsrc.dstirq = sci_irq;
@@ -268,16 +271,16 @@ pub fn setup_mptable(
     }
     let pci_irq_base = super::X86_64_IRQ_BASE as u8;
     // Insert PCI interrupts after platform IRQs.
-    for (i, pci_irq) in pci_irqs.iter().enumerate() {
+    for (address, irq_num, irq_pin) in pci_irqs.iter() {
         let size = mem::size_of::<mpc_intsrc>();
         let mut mpc_intsrc = mpc_intsrc::default();
         mpc_intsrc.type_ = MP_INTSRC as u8;
         mpc_intsrc.irqtype = mp_irq_source_types_mp_INT as u8;
         mpc_intsrc.irqflag = MP_IRQDIR_DEFAULT as u16;
-        mpc_intsrc.srcbus = PCI_BUS_ID;
-        mpc_intsrc.srcbusirq = (pci_irq.0 as u8 + 1) << 2 | pci_irq.1.to_mask() as u8;
+        mpc_intsrc.srcbus = address.bus;
+        mpc_intsrc.srcbusirq = address.dev << 2 | irq_pin.to_mask() as u8;
         mpc_intsrc.dstapic = ioapicid;
-        mpc_intsrc.dstirq = pci_irq_base + i as u8;
+        mpc_intsrc.dstirq = u8::try_from(*irq_num).map_err(|_| Error::WriteMpcIntsrc)?;
         mem.write_obj_at_addr(mpc_intsrc, base_mp)
             .map_err(|_| Error::WriteMpcIntsrc)?;
         base_mp = base_mp.unchecked_add(size as u64);
@@ -290,7 +293,7 @@ pub fn setup_mptable(
         mpc_intsrc.type_ = MP_INTSRC as u8;
         mpc_intsrc.irqtype = mp_irq_source_types_mp_INT as u8;
         mpc_intsrc.irqflag = MP_IRQDIR_DEFAULT as u16;
-        mpc_intsrc.srcbus = ISA_BUS_ID;
+        mpc_intsrc.srcbus = isa_bus_id;
         mpc_intsrc.srcbusirq = i as u8;
         mpc_intsrc.dstapic = ioapicid;
         mpc_intsrc.dstirq = i as u8;
@@ -305,7 +308,7 @@ pub fn setup_mptable(
         mpc_lintsrc.type_ = MP_LINTSRC as u8;
         mpc_lintsrc.irqtype = mp_irq_source_types_mp_ExtINT as u8;
         mpc_lintsrc.irqflag = MP_IRQDIR_DEFAULT as u16;
-        mpc_lintsrc.srcbusid = ISA_BUS_ID;
+        mpc_lintsrc.srcbusid = isa_bus_id;
         mpc_lintsrc.srcbusirq = 0;
         mpc_lintsrc.destapic = 0;
         mpc_lintsrc.destapiclint = 0;
@@ -320,7 +323,7 @@ pub fn setup_mptable(
         mpc_lintsrc.type_ = MP_LINTSRC as u8;
         mpc_lintsrc.irqtype = mp_irq_source_types_mp_NMI as u8;
         mpc_lintsrc.irqflag = MP_IRQDIR_DEFAULT as u16;
-        mpc_lintsrc.srcbusid = ISA_BUS_ID;
+        mpc_lintsrc.srcbusid = isa_bus_id;
         mpc_lintsrc.srcbusirq = 0;
         mpc_lintsrc.destapic = 0xFF; // Per SeaBIOS
         mpc_lintsrc.destapiclint = 1;